Skip to content

gh-154013: Add free-threading test for shared struct.iter_unpack#154141

Closed
johng wants to merge 1 commit into
python:mainfrom
johng:struct-iter-unpack-fix
Closed

gh-154013: Add free-threading test for shared struct.iter_unpack#154141
johng wants to merge 1 commit into
python:mainfrom
johng:struct-iter-unpack-fix

Conversation

@johng

@johng johng commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Leverage _Py_atomic_compare_exchange_ssize to "claim" a single index position for each thread

Leverage `_Py_atomic_compare_exchange_ssize` to "claim"
a single index position for each thread
@johng johng changed the title Add free-threading test for shared struct.iter_unpack gh-154013: Add free-threading test for shared struct.iter_unpack Jul 19, 2026
@johng

johng commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

(AI report on the performance / overall change)

Free-threaded (--disable-gil) memory-safety fix for struct.Struct.iter_unpack.
A single iter_unpack iterator shared by multiple threads previously hit a
use-after-free / double buffer release on exhaustion and an out-of-bounds read
from a non-atomic index update. This document records the fix, the
ThreadSanitizer verification, and the performance impact.

The fix

On the free-threaded build, unpackiter_iternext claims each record's slot with
a lock-free compare-and-swap on self->index, and defers releasing the
buffer/Struct to unpackiter_dealloc (which runs only at refcount 0, when no
thread can still be iterating). The CAS gives each record to exactly one thread
(no duplicate, no skip); every offset is bounds-checked before the read; index
is monotonic so there is no ABA hazard. This mirrors the existing
itertools.count fast path, which claims its counter with the same
_Py_atomic_compare_exchange_ssize idiom. The GIL build is unchanged — it keeps
the original plain increment and eager release. (See the PR diff for the code.)

ThreadSanitizer verification

Build: --with-thread-sanitizer --disable-gil --with-pydebug. Reproducer: 8
threads drain one shared iter_unpack to exhaustion, 5000 rounds.

before fix after fix
TSan data race in unpackiter_iternext + SEGV 0 races, clean exit
$ TSAN_OPTIONS="halt_on_error=1" PYTHON_GIL=0 ./python.exe repro.py
survived this run          # after fix: multiple full rounds, exit 0, no warnings

Performance

Environment. Apple Silicon (arm64), macOS; 6 performance cores / 18 logical.
CPython 3.16.0a0 free-threading, clang, release build (--disable-gil, no
pydebug, no TSan). PYTHON_GIL=0. Throughput = best of 7 repeats.

Single-threaded hot path (ns per element, min of 12 runs)

Lower is better. Relative overhead is largest for a 1-field struct (least work
to hide a per-element atomic behind).

format fix disabled fix enabled (CAS)
1×i 10.46 10.44
4×i 16.29 16.29
16×i 36.92 36.73

No measurable single-threaded regression (within run-to-run noise).

Multi-threaded throughput (Melem/s, higher is better)

Three workloads, three variants — all measured in one run:

  • workloads: single (1 thread); independent (N threads, each its own
    iterator — real free-threaded parallelism, safe with and without the fix);
    shared (N threads draining one iterator — the case the fix protects, which
    crashes without it).
  • variants: fix disabled (native iterator, unpatched); fix enabled
    (native, CAS patch); serialize_iterator
    (threading.serialize_iterator(iter_unpack(...)) — the suggested fallback,
    which takes a Python-level lock around every __next__).
workload fix disabled fix enabled (CAS) serialize_iterator
single 93.2 91.0 16.5
independent · 1t 90.6 89.9 16.4
independent · 2t 173.6 174.4 32.5
independent · 4t 345.0 344.5 64.1
independent · 8t 465.0 498.9 88.0
shared · 1t 💥 crash 78.8 11.2
shared · 2t 💥 crash 31.3 8.8
shared · 4t 💥 crash 29.7 6.3
shared · 8t 💥 crash 14.5 6.8

Takeaways.

  • The CAS fix has no measurable cost on unshared iteration. single and
    independent match the unpatched build within ±3% run-to-run noise (deltas
    scatter both directions); parallel scaling is unchanged (~90 → ~470–500
    Melem/s either way). The per-element CAS is invisible when the iterator is not
    shared — i.e. in every normal case.
  • Shared: crash → correct. The workload that segfaulted now runs safely.
    It is slow under contention (all threads funnel through one cursor), but that
    is inherent to sharing one iterator; independent iterators give ~30× the
    throughput. Nobody shares an iterator for speed.
  • serialize_iterator is correct but far slower. Its Python-level lock
    costs ~5.5× on single (16.5 vs 91) and, because that per-__next__
    overhead is paid on every element, it caps independent throughput at ~88
    Melem/s vs the native fix's ~499 (8t). Even in the shared case it exists to
    handle, it is 2–7× slower than the native fix. The native fix dominates it
    everywhere while adding no cost to unshared use.

@ZeroIntensity

Copy link
Copy Markdown
Member

This is not worth the complexity. See the issue.

@johng

johng commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

@ZeroIntensity this pattern seemed quite common and the solution to use the threaded serializer seems to come with a very high performance penalty

@ZeroIntensity

Copy link
Copy Markdown
Member

Do you have real-world examples where people would benefit from fast, concurrent iteration here? The primary issue with the concurrent iteration pattern is that the order in which items are returned to each thread is non-deterministic; a thread has to somehow know where it is in the iterator (which is impossible to do safely without external locking) or not care about the position of the iterator, which is not a common occurrence, and there are usually better patterns (such as distributing items to worker threads sequentially, rather than letting each of them access the iterator).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants