X Tutup
Skip to content

gh-132617: Fix dict.update() mutation check#134815

Merged
colesbury merged 3 commits intopython:mainfrom
colesbury:gh-132617-dict-update
Jun 16, 2025
Merged

gh-132617: Fix dict.update() mutation check#134815
colesbury merged 3 commits intopython:mainfrom
colesbury:gh-132617-dict-update

Conversation

@colesbury
Copy link
Contributor

@colesbury colesbury commented May 27, 2025 •
edited by bedevere-app bot
Loading

Use ma_used instead of ma_keys->dk_nentries for modification check so that we only check if the dictionary is modified, not if new keys are added to a different dictionary that shared the same keys object.

@colesbury
pythongh-132617: Fix dict.update() mutation check
733dd33 
Use `ma_used` instead of `ma_keys->dk_nentries` for modification check
so that we only check if the dictionary is modified, not if new keys are
added to a different dictionary that shared the same keys object.
@colesbury colesbury added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels May 27, 2025
@bedevere-app bedevere-app bot mentioned this pull request May 27, 2025
Closed
@colesbury colesbury requested a review from mpage May 27, 2025 20:45
@colesbury colesbury changed the title gh-132617: Fix dict.update() mutation check gh-132617: Fix dict.update() mutation check May 27, 2025
mpage
mpage reviewed May 28, 2025
View reviewed changes
methane
methane reviewed May 28, 2025
View reviewed changes
Objects/dictobject.c
}

Py_ssize_t orig_size = other->ma_keys->dk_nentries;
Py_ssize_t orig_size = other->ma_used;
Copy link
Member

@methane methane May 28, 2025

Choose a reason for hiding this comment

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

ma_used is too weak. It cannot detect del d["a"]; d["b"] = None.

For better check:

  • For all table, check other->ma_keys is not changed.
  • For combined table, check other->ma_keys->dk_nentries is not changed.
  • For split table, check other->ma_values->size is not changed.

Copy link
Contributor Author

@colesbury colesbury May 28, 2025

Choose a reason for hiding this comment

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

It's the same condition we use in iterators and elsewhere. I don't think we should strengthen any of the checks. These are heuristic checks, they're not going to be perfect, but if we decide to do so, I think we should do that in a subsequent PR and not backport that change.

cpython/Objects/dictobject.c

Lines 5169 to 5174 in 469a564

if (di->di_used != d->ma_used) {
PyErr_SetString(PyExc_RuntimeError,
"dictionary changed size during iteration");
di->di_used = -1; /* Make this state sticky */
return NULL;
}

@colesbury colesbury requested review from methane and mpage June 3, 2025 21:31
@colesbury colesbury merged commit d8994b0 into python:main Jun 16, 2025
43 checks passed
@miss-islington-app
Copy link

miss-islington-app bot commented Jun 16, 2025

Thanks @colesbury for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@colesbury colesbury deleted the gh-132617-dict-update branch June 16, 2025 16:55
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 16, 2025
@colesbury @miss-islington
pythongh-132617: Fix dict.update() mutation check (pythongh-134815)
135b7f1 
Use `ma_used` instead of `ma_keys->dk_nentries` for modification check
so that we only check if the dictionary is modified, not if new keys are
added to a different dictionary that shared the same keys object.
(cherry picked from commit d8994b0)

Co-authored-by: Sam Gross <colesbury@gmail.com>
@miss-islington-app
Copy link

miss-islington-app bot commented Jun 16, 2025

Sorry, @colesbury, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker d8994b0a77cc9821772d05db00a6ab23382fa17d 3.13

@bedevere-app
Copy link

bedevere-app bot commented Jun 16, 2025

GH-135581 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Jun 16, 2025
@bedevere-app
Copy link

bedevere-app bot commented Jun 16, 2025

GH-135582 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Jun 16, 2025
colesbury added a commit to colesbury/cpython that referenced this pull request Jun 16, 2025
@colesbury
[3.13] pythongh-132617: Fix dict.update() mutation check (pythongh-…
9f03ea6 
…134815)

Use `ma_used` instead of `ma_keys->dk_nentries` for modification check
so that we only check if the dictionary is modified, not if new keys are
added to a different dictionary that shared the same keys object.
(cherry picked from commit d8994b0)

Co-authored-by: Sam Gross <colesbury@gmail.com>
colesbury added a commit that referenced this pull request Jun 16, 2025
@miss-islington @colesbury
[3.14] gh-132617: Fix dict.update() mutation check (gh-134815) (gh-…
15f7bd4 
…135581)

Use `ma_used` instead of `ma_keys->dk_nentries` for modification check
so that we only check if the dictionary is modified, not if new keys are
added to a different dictionary that shared the same keys object.
(cherry picked from commit d8994b0)

Co-authored-by: Sam Gross <colesbury@gmail.com>
colesbury added a commit that referenced this pull request Jun 16, 2025
@colesbury
[3.13] gh-132617: Fix dict.update() mutation check (gh-134815) (gh-…
f33a5e8 
…135582)

Use `ma_used` instead of `ma_keys->dk_nentries` for modification check
so that we only check if the dictionary is modified, not if new keys are
added to a different dictionary that shared the same keys object.
(cherry picked from commit d8994b0)
lkollar pushed a commit to lkollar/cpython that referenced this pull request Jun 19, 2025
@colesbury @lkollar
pythongh-132617: Fix dict.update() mutation check (pythongh-134815)
9b67994 
Use `ma_used` instead of `ma_keys->dk_nentries` for modification check
so that we only check if the dictionary is modified, not if new keys are
added to a different dictionary that shared the same keys object.
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
@colesbury @Pranjal095
pythongh-132617: Fix dict.update() mutation check (pythongh-134815)
2f95b70 
Use `ma_used` instead of `ma_keys->dk_nentries` for modification check
so that we only check if the dictionary is modified, not if new keys are
added to a different dictionary that shared the same keys object.
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
@colesbury @taegyunkim
pythongh-132617: Fix dict.update() mutation check (pythongh-134815)
1d2a82d 
Use `ma_used` instead of `ma_keys->dk_nentries` for modification check
so that we only check if the dictionary is modified, not if new keys are
added to a different dictionary that shared the same keys object.
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025
@colesbury @Agent-Hellboy
pythongh-132617: Fix dict.update() mutation check (pythongh-134815)
4ec74d8 
Use `ma_used` instead of `ma_keys->dk_nentries` for modification check
so that we only check if the dictionary is modified, not if new keys are
added to a different dictionary that shared the same keys object.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mpage mpage mpage approved these changes

@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

@methane methane Awaiting requested review from methane methane is a code owner

Assignees

@colesbury colesbury

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@colesbury @methane @mpage
X Tutup