Fix dict race condition by youknowone · Pull Request #6720 · RustPython/RustPython

youknowone · 2026-01-13T10:00:10Z

Summary by CodeRabbit

New Features
- Enhanced dictionary iteration capabilities with more efficient and thread-safe access to dictionary keys, values, and items during atomic operations.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2026-01-13T10:00:16Z

📝 Walkthrough

Walkthrough

Three new vectorized accessor methods are added to PyDict (keys_vec, values_vec, items_vec) to expose dictionary contents as atomic snapshots. Corresponding methods are implemented in the underlying DictInner and Dict generic types. The VM's element extraction logic is extended to handle iteration over dict view types.

Changes

Cohort / File(s)	Summary
PyDict Accessor Methods `crates/vm/src/builtins/dict.rs`	Added three new public methods: `keys_vec()`, `values_vec()`, and `items_vec()` to return thread-safe vectorized snapshots of dictionary keys, values, and key-value pairs respectively.
Generic Dict Container Methods `crates/vm/src/dict_inner.rs`	Added public methods to `DictInner<T>` and `Dict<T>` to expose `values()` and `items()` as vectors, enabling retrieval of all values and key-value pairs in insertion order from the underlying entries collection.
VM Element Extraction Logic `crates/vm/src/vm/mod.rs`	Extended `extract_elements_with` to support atomic iteration over dict_keys_type, dict_values_type, and dict_items_type by obtaining vectorized snapshots and mapping elements through the provided function. Dict items are converted to Python tuples before applying the function.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A dict reveals its treasures three,
Keys and values, wild and free!
Vectors gather what was locked,
Snapshots safe—our VM's stocked!
Thread-safe access, swift and clean,
Finest dict views ever seen! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'Fix dict race condition' is vague and does not clearly describe the actual changes, which add new vectorized accessor methods to PyDict for thread-safe element extraction in iteration loops.	Consider a more specific title like 'Add thread-safe dict accessor methods for iteration' that better reflects the main change of introducing keys_vec, values_vec, and items_vec methods.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7e47212 and 917c9f6.

📒 Files selected for processing (3)

crates/vm/src/builtins/dict.rs
crates/vm/src/dict_inner.rs
crates/vm/src/vm/mod.rs

🧰 Additional context used

📓 Path-based instructions (1)

**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

crates/vm/src/builtins/dict.rs
crates/vm/src/dict_inner.rs
crates/vm/src/vm/mod.rs

🧠 Learnings (1)

📚 Learning: 2025-12-27T14:03:49.034Z

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-27T14:03:49.034Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust

Applied to files:

crates/vm/src/vm/mod.rs

🧬 Code graph analysis (2)

crates/vm/src/dict_inner.rs (1)

crates/vm/src/builtins/dict.rs (2)

values (405-407)

items (410-412)

crates/vm/src/vm/mod.rs (2)

crates/vm/src/builtins/dict.rs (4)

dict (774-774)

keys (400-402)

values (405-407)

items (410-412)

crates/vm/src/dict_inner.rs (3)

keys (548-554)

values (556-562)

items (564-570)

🔇 Additional comments (5)

crates/vm/src/dict_inner.rs (1)

556-570: LGTM! Consistent implementation following established patterns.

The new values() and items() methods correctly mirror the existing keys() method pattern (lines 548-554): acquire read lock, filter-map entries, collect to Vec. This ensures atomic snapshots while the lock is held.

crates/vm/src/vm/mod.rs (3)

815-837: LGTM! Atomic snapshots for dict views prevent race conditions.

The implementation correctly uses the new keys_vec(), values_vec(), and items_vec() methods to get atomic snapshots before iteration, solving the race condition issue.

23-27: LGTM!

Import additions are correctly organized to support the new dict view type checks.

834-836: This optimization is not valid and would not compile.

The Context::new_tuple method requires Vec<PyObjectRef>, and arrays [T; N] do not implement IntoPyTuple. While VirtualMachine::new_tuple accepts impl IntoPyTuple, the code uses self.ctx.new_tuple(), which calls the Context version directly. The suggestion to change vec![k, v] to [k, v] would fail to compile.

Likely an incorrect or invalid review comment.

crates/vm/src/builtins/dict.rs (1)

65-81: LGTM! Well-documented atomic snapshot accessors.

The _vec suffix appropriately distinguishes these methods from the existing view-returning methods (lines 400-412). The doc comments clearly explain the thread-safety guarantees.

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

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Fix dict race condition

917c9f6

youknowone marked this pull request as ready for review January 13, 2026 10:39

youknowone merged commit e91bc11 into RustPython:main Jan 13, 2026
12 of 13 checks passed

youknowone deleted the dict-race branch January 13, 2026 12:27

terryluan12 pushed a commit to terryluan12/RustPython that referenced this pull request Jan 15, 2026

Fix dict race condition (RustPython#6720)

e40eac4

This was referenced Feb 27, 2026

Fix dict race condition #7239

Merged

Fix thread-safety in GC, type cache, and instruction cache #7355

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix dict race condition#6720

Fix dict race condition#6720
youknowone merged 1 commit intoRustPython:mainfrom
youknowone:dict-race

youknowone commented Jan 13, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jan 13, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

youknowone commented Jan 13, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jan 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

youknowone commented Jan 13, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jan 13, 2026 •

edited

Loading