Skip to content

Align itertools source and callback lifetimes - #9721

Merged
youknowone merged 5 commits into
RustPython:mainfrom
youknowdot:fix/itertools-lifecycle-controls-20261104
Nov 8, 2026
Merged

youknowone merged 5 commits into
RustPython:mainfrom
youknowdot:fix/itertools-lifecycle-controls-20261104

Conversation

@youknowdot

@youknowdot youknowdot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Align itertools callback state and source ownership with CPython 3.15, rebased onto main 3d0f6fc659b02733722ec658eb2be7c5a9288c68.

  • Read accumulate's running total after source callbacks and release arithmetic operands before publishing the replacement total.
  • Release converted non-exact-tuple starmap containers before calling the function; retain exact tuples through the call.
  • Detach cycle's exhausted source after publishing replay state and preserve the outer call's replay index across finalizer reentry.
  • Clear pairwise sources and previous items in the appropriate first/second-input failure order.
  • Detach nonfinal exhausted zip_longest sources while retaining the final/error source and protecting the active count with its optional slots.

State locks

Cycle's source and saved items now share its existing PyRwLock<CycleState>. Pairwise's source and old item share its existing PyRwLock<PairwiseState>. This removes both additional source mutexes while retaining typed PyIter fields. No extra PyObjectCell/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:

  • Normal commit hooks, rustfmt and diff checks passed.
  • Workspace tests and all-target Clippy with -D warnings passed.
  • Separate C-API tests (149 passed, 2 ignored) and Clippy with -D warnings passed.
  • Release test_itertools: 137 tests run, 22 skipped, passed.
  • Existing cycle/zip_longest threading tests: 2 passed.
  • Full changed itertools snippet, cycle threading snippet and focused pytest wrapper: passed.
  • Added finalizer-reentry/source-release regressions fail on the preserved pre-PR binary and pass with this revision. Existing Lib tests cover pairwise callback reentry.

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 by PyIterReturn::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.

@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Caution

Review failed

The pull request is closed.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

@codspeed

codspeed Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 4 skipped benchmarks1


Comparing youknowdot:fix/itertools-lifecycle-controls-20261104 (7f770ad) with main (244569c)2

Open in CodSpeed

Footnotes

  1. 4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

  2. No successful run was found on main (f1f38c1) during the generation of this report, so 244569c was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

@youknowdot
youknowdot force-pushed the fix/itertools-lifecycle-controls-20261104 branch 2 times, most recently from 7f770ad to 3930446 Compare November 7, 2026 14:11
youknowdot and others added 5 commits November 7, 2026 23:13
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
@youknowdot
youknowdot force-pushed the fix/itertools-lifecycle-controls-20261104 branch from 3930446 to 3cb2363 Compare November 7, 2026 15:05
@youknowone
youknowone marked this pull request as ready for review November 8, 2026 02:32
@youknowone
youknowone merged commit cb1841a into RustPython:main Nov 8, 2026
20 checks passed
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 8, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-08T02:40:26.328246Z 3cb2363 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants