Skip to content

Fix dict race condition#7239

Merged
youknowone merged 2 commits intoRustPython:mainfrom
youknowone:dict
Feb 28, 2026
Merged

Fix dict race condition#7239
youknowone merged 2 commits intoRustPython:mainfrom
youknowone:dict

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Feb 27, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Improved dictionary lookup stability: the lookup now reliably resets its probing state when the underlying storage changes, preventing stale probe state from causing incorrect or inconsistent results during resizes. This ensures lookups remain correct and robust as the internal storage grows or shrinks.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d199927 and caa4fe9.

📒 Files selected for processing (1)
  • .github/workflows/ci.yaml

📝 Walkthrough

Walkthrough

DictInner::lookup now detects when the underlying indices array is resized during probing and restarts the probe generator with the new mask, clearing any cached free-slot marker. CI workflow timeouts were increased for several jobs.

Changes

Cohort / File(s) Summary
Hash Table Probing Resize Handling
crates/vm/src/dict_inner.rs
DictInner::lookup now tracks the current indices mask during probing; if the mask changes (resize), it reinitializes GenIndexes and clears stored free-slot state to avoid stale probe state.
CI Timeout Adjustments
.github/workflows/ci.yaml
Increased timeout-minutes for multiple CI jobs (Linux 45→60, macOS 45→50, Windows 45→50). No workflow steps or logic changed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 In burrows of bytes I nimbly hop,
If masks shift mid-probe I stop and swap,
Reinit my hops, clear crumbs on the floor,
Now lookups prance true — and CI waits a bit more. ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix dict race condition' directly addresses the main change in the PR, which involves fixing a race condition in DictInner::lookup by properly handling mask changes during resizes.
Docstring Coverage ✅ Passed Docstring coverage is 100.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 unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

@youknowone youknowone marked this pull request as ready for review February 27, 2026 08:12
@youknowone youknowone merged commit 0e48ca9 into RustPython:main Feb 28, 2026
33 of 35 checks passed
@youknowone youknowone deleted the dict branch February 28, 2026 01:35
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.

1 participant