All commit authors signed the Contributor License Agreement.

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

Trivial self-reference is broken. Pushing fix in a moment

@serhiy-storchaka - should I force push to trigger a retest? There was a flakey upstream apt failure

I re-ran it.

Try to load b'\xa9\x01[\x01\x00\x00\x00r\x00\x00\x00\x00'. Currently it works, but with this PR it fails.

Try to load b'\xa9\x01[\x01\x00\x00\x00r\x00\x00\x00\x00'. Currently it works, but with this PR it fails.

Good catch! I found a few other weird cases that regress:

  - Direct self-referential tuple: t = (t,)
    b'\xa9\x01r\x00\x00\x00\x00'
  - Tuple through dict value: ({'x': t},)
    b'\xa9\x01\xfb\xda\x01xr\x00\x00\x00\x000'
  - Tuple through list with multiple refs back to the same tuple
    b'\xa9\x01[\x02\x00\x00\x00r\x00\x00\x00\x00r\x00\x00\x00\x00'
  - Non-tuple root, but inner tuple is still the back-ref target
    Shape: [( [t], )]
    b'\xdb\x01\x00\x00\x00\xa9\x01\xdb\x01\x00\x00\x00r\x01\x00\x00\x00'

1. @serhiy-storchaka , would you want me to improve the test cases in ./Lib/test/test_marshal.py to add these all or leave that as-is?

2. I assume you had already thought of this too, which was the point of your example, but perhaps the real solution is to avoid changing r_object() and add the guard in tuple_hash() instead. If so, would you go with ValueError or SystemError?

There are many other examples, it was just the simplest case. It seems you searched examples by some kind of fuzz generator (or just asked LLM?). You cannot be sure that that you will cover all cases. But they can be found by simple combinatorics. I'll add more tests.

Some of cases can be created in Python, others (like self-containing tuple) can only be artificially constructed marshal data. We should support the former, because they can occur in real program. We should prevent crashes and creation of inconsistent data structures when loading the latter.

I am not sure that changing tuple.__hash__() is a good solution. First, it will add some overhead in normal cases and can hide bugs in other parts. Second, we have similar issues with frozenset and frozendict which cannot be solved by that method. This may be a complex issue.

There are many other examples, it was just the simplest case. It seems you searched examples by some kind of fuzz generator (or just asked LLM?). You cannot be sure that that you will cover all cases. But they can be found by simple combinatorics. I'll add more tests.

I ran a simple test with itertools and typing feeding spawning sub-Pythons and then asked Opus 4.6 to expand from there. You're right about the scope of the issue.

Some of cases can be created in Python, others (like self-containing tuple) can only be artificially constructed marshal data. We should support the former, because they can occur in real program. We should prevent crashes and creation of inconsistent data structures when loading the latter.

I am not sure that changing tuple.__hash__() is a good solution. First, it will add some overhead in normal cases and can hide bugs in other parts. Second, we have similar issues with frozenset and frozendict which cannot be solved by that method. This may be a complex issue.

I'm beginning to appreciate your last sentence here...I think a proper fix for this would likely end up causing a performance impact.

If you want a PR to expand the test coverage so no one makes the same mistake I did, I'd be happy to submit it for now and defer the design questions to you all.

See #148700

There are now tests for such abnormal cases, one of them is disabled by if False due to leaks. See test_reference_loop_tuple, test_loads_abnormal_reference_loops. @mjbommar, could you please fix these tests? I suggest also to use the same methods that was used to prevent creation of invalid marshal data for code objects, slices and frozendicts in #148698 -- use w_complete() and add the check for tuples in w_ref().