`test_super_traverse_early_return_does_not_abort` is flaky on free-threaded builds (~30% of runs)

[jonasdedden]

Bug Description

test_super_traverse_early_return_does_not_abort (tests/test_gc.rs:829, added in #6206) fails intermittently on the free-threaded CI job:

thread 'test_super_traverse_early_return_does_not_abort' panicked at tests/test_gc.rs:850:9:
child __traverse__ ran despite the super-type traverse returning non-zero

It reproduces on main at 1ed13a0, without any local modification:

build	failure rate
free-threaded 3.15.0b4, --features=abi3t,full,multiple-pymethods --no-default-features	12 / 40 runs
GIL-enabled 3.14.6, --features=full,multiple-pymethods	0 / 40 runs

This is not a rare flake.

Steps to Reproduce

uv python install 3.15t
export PYO3_PYTHON=$(uv python find 3.15t)
cargo test --no-fail-fast --features=abi3t,full,multiple-pymethods --no-default-features --test test_gc
# re-run a few times, or narrow to the minimal pair:
#   <test binary> --exact test_super_traverse_early_return_does_not_abort dict_cycle_collected_without_traverse

Backtrace

The panic itself, from a local run on main (RUST_BACKTRACE=1) — frame for frame the same as CI:

thread 'test_super_traverse_early_return_does_not_abort' (1469631) panicked at tests/test_gc.rs:850:9:
child __traverse__ ran despite the super-type traverse returning non-zero
stack backtrace:
   0: __rustc::rust_begin_unwind
             at /rustc/ac68faa20c58cbccd01ee7208bf3b6e93a7d7f96/library/std/src/panicking.rs:689:5
   1: core::panicking::panic_fmt
             at /rustc/ac68faa20c58cbccd01ee7208bf3b6e93a7d7f96/library/core/src/panicking.rs:80:14
   2: test_gc::test_super_traverse_early_return_does_not_abort::{{closure}}
             at ./tests/test_gc.rs:850:9
   3: pyo3::marker::Python::attach
             at ./src/marker.rs:419:9
   4: test_gc::test_super_traverse_early_return_does_not_abort
             at ./tests/test_gc.rs:830:5
   5: test_gc::test_super_traverse_early_return_does_not_abort::{{closure}}
             at ./tests/test_gc.rs:829:53

That backtrace only shows the assertion site, not the cause — the traversal that set the flag happened earlier, on a different thread. Instrumenting TraverseChild::__traverse__ to dump a backtrace whenever it runs (and re-running with --nocapture, since the output otherwise belongs to the other test and gets swallowed) captures the actual chain:

>>> TraverseChild::__traverse__ ran on thread Some("test_cycle_clear")
   0: test_gc::TraverseChild::__traverse__
             at ./tests/test_gc.rs:827:13
   1: pyo3::impl_::pymethods::traverse_impl::{{closure}}
             at ./src/impl_/pymethods.rs:441:49
   ...
   7: pyo3::impl_::pymethods::traverse_impl
             at ./src/impl_/pymethods.rs:441:11
   8: pyo3::impl_::pymethods::_call_traverse
             at ./src/impl_/pymethods.rs:372:27
   9: test_gc::TraverseChild::__pymethod_traverse__
             at ./tests/test_gc.rs:819:1
  10: update_refs
  11: mi_heap_visit_blocks
  12: gc_visit_heaps
  13: gc_collect_main
  14: gc_collect
  ...
  17: PyEval_EvalCode
  18: <pyo3::instance::Bound<PyCode> as PyCodeMethods>::run
             at ./src/types/code.rs:133:13
  19: pyo3::marker::Python::run
             at ./src/marker.rs:643:14
  20: test_gc::DropCheck::assert_drops_with_gc::{{closure}}
             at ./tests/test_gc.rs:107:20

Read bottom-up: assert_drops_with_gc runs gc.collect() on the test_cycle_clear thread
(frames 20-14), the free-threaded collector's update_refs phase visits every tracked object
(frames 13-10), reaches our object and runs traverse_impl (frames 9-7). update_refs visits with
a visitproc that always returns 0, so the super-traverse does not return non-zero, nothing returns
early, and the child's __traverse__ body runs (frames 1-0) and sets the global flag.

Two such interfering traversals were recorded in the failing run.

Your operating system and version

Linux (CachyOS, kernel 7.1.3)

Your Python version (python --version)

3.15.0b4 free-threading build (fails) / 3.14.6 GIL-enabled (passes)

Your Rust version (rustc --version)

rustc 1.96.0

Your PyO3 version

0.29

How did you install python? Did you use a virtualenv?

uv

Additional Info

Root Cause

The test asserts on a process-global AtomicBool that any traversal of the object sets:

static CHILD_TRAVERSED: AtomicBool = AtomicBool::new(false);

impl TraverseChild {
    fn __traverse__(&self, _visit: PyVisit<'_>) -> Result<(), PyTraverseError> {
        CHILD_TRAVERSED.store(true, Ordering::SeqCst);
        Ok(())
    }
}

traverse_impl (src/impl_/pymethods.rs:397) only skips the child's __traverse__ when the super-type traverse returns non-zero:

let super_retval = unsafe { call_super_traverse(slf, visit, arg, current_traverse) };
if super_retval != 0 {
    return super_retval;
}
// ... eventually calls impl_(), i.e. TraverseChild::__traverse__

Only gc.get_referrers()'s referrersvisit returns non-zero. Every ordinary collection visits
with visit_decref, which always returns 0, so a plain gc.collect() runs the child's
__traverse__ to completion and sets the flag.

The test therefore asserts "no garbage collection touched this object between the store(false) and
the assertion" — which nothing in the test guarantees.

Proof of the mechanism

Inserting one collection into the window makes it fail deterministically, on a GIL build, running
the test alone:

CHILD_TRAVERSED.store(false, Ordering::SeqCst);
py.import("gc").unwrap().call_method0("collect").unwrap();   // added

EXPERIMENT: after a plain gc.collect(), CHILD_TRAVERSED = true
panicked at tests/test_gc.rs:854:9: child __traverse__ ran despite ...

Why free-threaded specifically

The collections come from other tests in the same binary. assert_drops_with_gc
(tests/test_gc.rs:99) runs up to 100
collections in a loop, and on the free-threaded build sleeps 5 ms per iteration — which stretches
those tests out to as much as 500 ms and widens the overlap:

for _ in 0..100 {
    if self.0.is_completed() { return; }
    Python::attach(|py| { py.run(c"import gc; gc.collect()", None, None).unwrap(); });
    #[cfg(Py_GIL_DISABLED)]
    std::thread::sleep(std::time::Duration::from_millis(5));
}

Nine tests in test_gc.rs call it. On a GIL build they cannot run Python concurrently with the
window, because Python::attach holds the GIL across the whole closure; on the free-threaded build
they run genuinely in parallel.

Isolating it confirms this exactly (free-threaded, 3.15.0b4):

selection	failure rate
test_super_traverse_early_return_does_not_abort alone	0 / 20
that test + dict_cycle_collected_without_traverse	15 / 20

Two tests are sufficient to reproduce it.

Suggested Fix

Don't involve the garbage collector at all. test_gc.rs already has the pattern for this:
unsendable_are_not_traversed_on_foreign_thread (tests/test_gc.rs:569) also asserts a negative about traversal, and does it by fetching tp_traverse with the file's get_type_traverse helper and calling it directly with its own visitproc. The file also already has visit_error, a visitproc returning non-zero — exactly the halt-early behaviour gc.get_referrers() was standing in
for.

Doing the same here makes the test deterministic, and lets it additionally assert the return value — which the current version never checks:

#[pyclass(extends = TraverseBase)]
struct TraverseChild {
    // Set by `__traverse__` so the test can assert whether the child's own traverse body ran.
    traversed: AtomicBool,
}

// The traverse is invoked directly rather than through `gc.get_referrers()`: a real collection
// visits with a `visitproc` which returns zero, so it runs the child traverse in full, and any
// concurrently running test may trigger one.
let traverse = unsafe { get_type_traverse(child.get_type().as_type_ptr()).unwrap() };
assert_ne!(
    unsafe { traverse(child.as_ptr(), visit_error, std::ptr::null_mut()) },
    0,
    "the non-zero from the super-type traverse should be propagated"
);
assert!(
    !child.borrow().traversed.load(Ordering::SeqCst),
    "child __traverse__ ran despite the super-type traverse returning non-zero"
);

The global static CHILD_TRAVERSED goes away entirely. A per-instance AtomicBool field is what the other traverse tests in this file already use (gc_during_borrow, traverse_cannot_be_hijacked), and the global was the only one of its kind in the file.

Verified on free-threaded 3.15.0b4:

	before	after
full test_gc suite	12 failures / 40 runs	0 failures / 60 runs
the two-test pair	15 failures / 20 runs	0 failures / 40 runs

It does not weaken the test — it strengthens it. Mutating traverse_impl to drop its early return (if false && super_retval != 0) still fails, and now fails on the return-value assertion, which is one step closer to the actual defect:

panicked at tests/test_gc.rs:848:9:
assertion `left != right` failed: the non-zero from the super-type traverse should be propagated

The original abort regression is still covered: an early return from traverse_impl with the PanicTrap still armed aborts the process on this path just the same, whether the non-zero comes from referrersvisit or from visit_error.

Alternative considered

Making the flag thread-local (thread_local! { static CHILD_TRAVERSED: Cell<bool> }) also takes the
failures to 0/60, because the interfering collections run on other tests' threads. But it keeps the
test coupled to GC timing and leaves a narrow same-thread window open (an automatic collection
triggered by the allocations inside py.import("gc") / call_method1). Calling tp_traverse
directly removes the dependency instead of filtering it, so that is the better fix.

[davidhewitt]

Please don't open issues like this. This is a mountain of LLM prose that wastes maintainers time to read.

Probably one well-written paragraph would have been enough to describe this.

[jonasdedden]

Sure, sorry! I'm more coming from a Python typing expert's perspective, and noticed this issue on another PR. I anyways was doing a LLM-powered investigation and thought it couldn't hurt to include the stuff it found out, especially since this is about a race issue. No ill intent here!

Happy to stick to whatever is best for you! 🙏 (i.e. just opening an extremely short issue or including the LLM investigation as a collapsible text paragraph or .txt file or whatever 😅)

[davidhewitt]

Thanks, appreciate that. We are still working on our AI policy (#6141) but the gist of what we generally want is that we communicate with the experts using the tools directly, not via the bot as an intermediary. I think most people would agree that bot output is typically way too verbose and any contributor not spending their time to reason about their bot output causes a 1:N amplification wasting many others' time reading that output instead.

I don't have a prescribed way that AI contributions must be presented; I think it should be clear in each contribution what is the most appropriate way to communicate with respect of others' time.