Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
733dd33
gh-132617: Fix `dict.update() ` mutation check
colesbury May 27, 2025
a6ed4ec
Update comment
colesbury May 28, 2025
5867156
Minor edit
colesbury May 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions Lib/test/test_dict.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,38 @@ def badgen():
['Cannot convert dictionary update sequence element #0 to a sequence'],
)

def test_update_shared_keys(self):
class MyClass: pass

# Subclass str to enable us to create an object during the
# dict.update() call.
class MyStr(str):
def __hash__(self):
return super().__hash__()

def __eq__(self, other):
# Create an object that shares the same PyDictKeysObject as
# obj.__dict__.
obj2 = MyClass()
obj2.a = "a"
obj2.b = "b"
obj2.c = "c"
return super().__eq__(other)

obj = MyClass()
obj.a = "a"
obj.b = "b"

x = {}
x[MyStr("a")] = MyStr("a")

# gh-132617: this previously raised "dict mutated during update" error
x.update(obj.__dict__)

self.assertEqual(x, {
MyStr("a"): "a",
"b": "b",
})

def test_fromkeys(self):
self.assertEqual(dict.fromkeys('abc'), {'a':None, 'b':None, 'c':None})
Expand Down
3 changes: 3 additions & 0 deletions Misc/NEWS.d/next/Core_and_Builtins/2025-05-27-20-29-00.gh-issue-132617.EmUfQQ.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix :meth:`dict.update` modification check that could incorrectly raise a
"dict mutated during update" error when a different dictionary was modified
that happens to share the same underlying keys object.
4 changes: 2 additions & 2 deletions Objects/dictobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3858,7 +3858,7 @@ dict_dict_merge(PyInterpreterState *interp, PyDictObject *mp, PyDictObject *othe
}
}

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

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

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;
}

Py_ssize_t pos = 0;
Py_hash_t hash;
PyObject *key, *value;
Expand Down Expand Up @@ -3892,7 +3892,7 @@ dict_dict_merge(PyInterpreterState *interp, PyDictObject *mp, PyDictObject *othe
if (err != 0)
return -1;

if (orig_size != other->ma_keys->dk_nentries) {
if (orig_size != other->ma_used) {
PyErr_SetString(PyExc_RuntimeError,
"dict mutated during update");
return -1;
Expand Down
Loading