Repository navigation
Align itertools source and callback lifetimes - #9721
youknowone merged 5 commits into
Conversation
|
Caution Review failedThe pull request is closed.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
Merging this PR will not alter performance
Comparing Footnotes
|
7f770ad to
3930446
Compare
Read current accumulate state after input callbacks and release starmap containers and exhausted iterator sources in reviewed reentrant order. Assisted-by: OpenAI Codex:model version not exposed
Preserve callback ownership, release ordering, and retained argument owners. Assisted-by: Codex:model-version-unavailable
Assisted-by: Codex:model-version-unavailable
Keep cycle and pairwise source ownership with their existing RwLock state instead of adding separate mutexes. Preserve callback and finalizer release ordering and add reentrant finalizer regressions. Assisted-by: OpenAI Codex
Use a native next callback so StopIteration traceback frames do not retain the source under test. Assisted-by: OpenAI Codex
3930446 to
3cb2363
Compare
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Summary
Align itertools callback state and source ownership with CPython 3.15, rebased onto main
3d0f6fc659b02733722ec658eb2be7c5a9288c68.State locks
Cycle's source and saved items now share its existing
PyRwLock<CycleState>. Pairwise's source and old item share its existingPyRwLock<PairwiseState>. This removes both additional source mutexes while retaining typedPyIterfields. No extraPyObjectCell/QSBR mechanism is introduced.ZipLongestState keeps one mutex because optional slots and the active count must be updated consistently. Callback snapshots are owned; callbacks and finalizing releases occur outside locks. Atomic cycle-index and pairwise-terminal publication are retained. Independent static concurrency review found no new lock-order or callback-under-lock issue. Concurrent next calls may still interleave; performance under contention was not benchmarked.
Validation
For
3cb2363d7551eeb5c0aabcac0512f9284050e1bf:-D warningspassed.-D warningspassed.test_itertools: 137 tests run, 22 skipped, passed.CPython 3.15.0 source was checked. No CPython 3.15 executable was available for this validation pass; supplemental CPython 3.12 native-next pairwise probes passed. Other platforms and hosted CI remain separate checks.
Limits
Cycle follows ordinary CPython source-release behavior; CPython's free-threaded build deliberately retains that source. Cached result-tuple reuse, generic tuple-conversion failure cleanup, and batched are not addressed.
A separate pre-existing iterator-protocol mismatch remains: Python-defined
__next__StopIteration exceptions are normalized byPyIterReturn::from_pyresult, discarding the original exception/traceback early. This can change source-finalizer ordering relative to CPython. The same exception-identity loss reproduces before this PR. Complete lifetime equivalence is not claimed; repairing raw-versus-normalized next semantics belongs in the shared iterator protocol rather than a mutex workaround here.AI-assisted contribution by OpenAI Codex, disclosed in commit trailers.