Skip to content

gh-158197: Fix split dict iteration during concurrent deletion - #158686

Merged
nascheme merged 3 commits into
python:mainfrom
nascheme:gh-158197-split-dict-iteration
Oct 20, 2026
Merged

gh-158197: Fix split dict iteration during concurrent deletion#158686
nascheme merged 3 commits into
python:mainfrom
nascheme:gh-158197-split-dict-iteration

Conversation

@nascheme

@nascheme nascheme commented Oct 3, 2026

Copy link
Copy Markdown
Member

Fix crashes and debug-build assertion failures in the free-threaded build when iterating over a split dictionary while another thread deletes entries or clears it. Publish insertion-order updates atomically so readers cannot observe an increased size before the corresponding order entry is initialized.

This is based on the patch attached to GH-158197 with some refinement. Note that AI agents (Claude and GPT) assisted in making and testing change.

@nascheme
pythongh-158197: Fix split dict iteration during concurrent deletion
6418581 
Read insertion order from the iterator's captured values snapshot instead
of re-reading ma_values and asserting against a concurrently changing
size. Retry under the dictionary lock when a value slot has been cleared.

Make insertion-order writes atomic and publish size changes with release
stores, pairing with the iterator's size load. Keep order-byte accesses
relaxed. Add _Py_atomic_store_uint8_release() and
FT_ATOMIC_STORE_UINT8_RELEASE() for this.
@nascheme nascheme added type-bug An unexpected behavior, bug, or error topic-free-threading needs backport to 3.14 bugs and security fixes needs backport to 3.15 bugs and security fixes labels Oct 3, 2026
@nascheme

nascheme commented Oct 10, 2026

Copy link
Copy Markdown
Member Author

The CI failure on Windows should be fixed by GH-158977, I think.

@bedevere-app bedevere-app Bot mentioned this pull request Oct 17, 2026
Open
@nascheme
nascheme requested a review from DinoV October 19, 2026 18:20
@nascheme

nascheme commented Oct 19, 2026

Copy link
Copy Markdown
Member Author

@DinoV mentioned we might investigate the performance impact of this. Split dicts are used for instance attributes. Therefore, we don't want to make the insert/update operation slower, if we can help it. That slowdown would be caused by using a release order store for size. I tried benchmarking this change on my MacBook M3 Pro. The impact seems small, to the point of being in the noise. That's with a micro-benchmark that does the affected operations on a split dict. I also ran a pyperformance compare, it shows no speed difference.

I also tried re-working the PR to use a relaxed store on size, instead of release. To make that work, we need to initialize the order array with zeros. With that change, dict init is a bit slower and the insert/del/clear operations are unchanged. So, no measurable speedup in the operation we care about (insert/update). I think using release, as this PR does, is the best approach.

DinoV
DinoV approved these changes Oct 20, 2026
View reviewed changes

@DinoV DinoV left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@nascheme
nascheme merged commit 4bb1dad into python:main Oct 20, 2026
56 checks passed
@miss-islington-app

miss-islington-app Bot commented Oct 20, 2026

Copy link
Copy Markdown

Thanks @nascheme for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14, 3.15.
🐍🍒⛏🤖

@bedevere-app

bedevere-app Bot commented Oct 20, 2026

Copy link
Copy Markdown

GH-159953 is a backport of this pull request to the 3.15 branch.

@miss-islington-app

miss-islington-app Bot commented Oct 20, 2026

Copy link
Copy Markdown

Sorry, @nascheme, I could not cleanly backport this to 3.14 due to a conflict.

Please backport manually with cherry_picker, see the devguide for more information.

cherry_picker 4bb1dadff42980eb133587f5798f20fb0354f039 3.14

@bedevere-app bedevere-app Bot removed the needs backport to 3.15 bugs and security fixes label Oct 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@methane methane methane approved these changes
@DinoV DinoV DinoV approved these changes
@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

Assignees

@nascheme nascheme

Labels

needs backport to 3.14 bugs and security fixes topic-free-threading type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nascheme @methane @DinoV