Repository navigation
Keep deque contents and bounds in coherent shared state - #9457
youknowone merged 1 commit into
Conversation
Use the existing creator-bound Python operation guard for deque mutations and index/copy, synchronize contents with maxlen, and preserve CPython callback ordering. Import the whole canonical free-threading collections test through update_lib. Assisted-by: OpenAI Codex: model version unavailable
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Deque state and mutations crates/vm/src/stdlib/_collections.rs |
Deque storage and accessors group items with maxlen. Extension and core mutation methods use operation guards and release removed objects after unlocking. |
Repetition, concatenation, and copying crates/vm/src/stdlib/_collections.rs |
Repetition, concatenation, and copying use deque snapshots and preserve maxlen. Allocation failures during repetition map to MemoryError. |
Search and indexed insertion crates/vm/src/stdlib/_collections.rs |
Index bounds use explicit conversion and saturation. Search and removal detect mutation during comparisons, and insert checks maxlen before changing the deque. |
Snapshots and iterator state crates/vm/src/stdlib/_collections.rs |
Representation and iterator setup read consistent deque state. Forward and reverse iterators share mutation and exhaustion handling while preserving their respective mutation-error behavior. |
Priority:
Estimated code review effort: 4 (Complex) | ~45 minutes
Change: Bug fix
Possibly related issues
- RustPython/RustPython#6839 — This PR imports
test_free_threading/test_collections.pyas one module-level step in the tracked library and test migration.
Possibly related PRs
- RustPython/RustPython#9427 — Both PRs address
MemoryErrorbehavior for sequence repetition. - RustPython/RustPython#9242 — Both PRs involve argument-count validation ordering relative to index conversion.
Suggested reviewers: youknowone, 1ndahous3
Merge Risk: 
0dc16
-
The deque synchronization changes have no established merge-blocking defect. Merge after normal checks; concurrent and stress behavior has not been independently validated.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: synchronizing deque contents and bounds in one coherent shared state. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 13 functions across 1 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- 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.
Comment @coderabbitai help to get the list of available commands.
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [ ] test: cpython/Lib/test/test_free_threading dependencies: dependent tests: (no tests depend on free_threading) Legend:
|
Codex Review SummaryThis 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
maxlenin one synchronized payload, removing the sharedmaxlenmutation through a cast.PyOperationfor writers and index/copy, preserving reentrant callbacks outside short payload locks.test_free_threading/test_collections.pythroughscripts/update_lib, pinned to CPythond220409a13b77de7a6c30abe16ea90db01528edc.This is independent of #9439. Its existing operation-guard prerequisite from #9449 is already on main.
CPython basis
_collectionsmodule.c: index lines 1262–1323, extend lines 479–574, copy lines 593–660, initialization lines 1755–1777, and iterator traversal lines 1969–2005 / 2120–2156.Validation
The exact two changed files were validated on main
b85582d57063c892fdf1839a495e47e5bf8e78bebefore the execution environment was replaced:cargo checkand strict Clippy passed.test_dequemethods passed: copy/copy_method, index regression 24913, maxlen attribute/zero, initialization, altered iterator data, and mutation of an initially empty iterator.Both changed files are restored byte-for-byte from that validated patch. The later main changes through
698c196b2e188c46772f8a3cf76164227282a645affect SQLite, regex, Windows overlapped cleanup, and an asyncio snippet; none overlap the deque implementation or operation guard. The binary was lost when the environment was replaced, so these are recorded earlier runs, not fresh execution on the final main base. Hosted CI must validate the published commit.The new whole free-threading module was not run locally. No full workspace/C-API suites, concurrent/GC/destructor/stress tests, or broad thread-safety claim are included. Remaining count/contains/richcompare, integer-width/error-text, GC/finalizer and cross-interpreter behavior is outside this change's scope.
AI assistance: OpenAI Codex (model version unavailable).
Summary by CodeRabbit
__index__and safe handling of out-of-range values.