Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 33 additions & 0 deletions Lib/test/test_ordered_dict.py
Original file line number Diff line number Diff line change
Expand Up @@ -879,6 +879,39 @@ def side_effect(self):
self.assertDictEqual(dict1, dict.fromkeys((0, 4.2)))
self.assertDictEqual(dict2, dict.fromkeys((0, Key(), 4.2)))

def test_issue148660_copy_clear_in_key_eq(self):
# gh-148660: od.copy() must not crash when a key's __eq__ clears od
# while copy() is inserting into the new dict.
armed = False
calls = 0
class Key:
def __hash__(self):
return 1
def __eq__(self, other):
nonlocal calls
if armed:
calls += 1
if calls == 2:
od.clear()
return self is other
od = self.OrderedDict()
od[Key()] = "v1"
od[Key()] = "v2"
armed = True
msg = "OrderedDict mutated during iteration"
self.assertRaisesRegex(RuntimeError, msg, od.copy)

def test_issue148660_copy_clear_in_subclass_getitem(self):
# gh-148660: od.copy() must not crash when a subclass __getitem__
# clears od.
class OD(self.OrderedDict):
def __getitem__(self, key):
od.clear()
return "v"
od = OD([(1, "v1"), (2, "v2")])
msg = "OrderedDict mutated during iteration"
self.assertRaisesRegex(RuntimeError, msg, od.copy)


@unittest.skipUnless(c_coll, 'requires the C version of the collections module')
class CPythonOrderedDictTests(OrderedDictTests,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix a crash in :meth:`collections.OrderedDict.copy` when a key's
``__eq__`` or a subclass method mutates the dict during the copy. Now
raises :exc:`RuntimeError` instead, as iteration does.
36 changes: 26 additions & 10 deletions Objects/odictobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -1251,36 +1251,52 @@ OrderedDict_copy_impl(PyObject *od)
if (od_copy == NULL)
return NULL;

/* The loop body may run arbitrary Python code which could mutate od and
free its nodes (gh-148660); detect that the same way __eq__ does. */
size_t state = _PyODictObject_CAST(od)->od_state;

if (PyODict_CheckExact(od)) {
_odict_FOREACH(od, node) {
PyObject *key = _odictnode_KEY(node);
PyObject *value = _odictnode_VALUE(node, od);
PyObject *key = Py_NewRef(_odictnode_KEY(node));
Py_hash_t hash = _odictnode_HASH(node);
PyObject *value = PyODict_GetItemWithError(od, key);
if (value == NULL) {
if (!PyErr_Occurred())
PyErr_SetObject(PyExc_KeyError, key);
Py_DECREF(key);
goto fail;
}
if (_PyODict_SetItem_KnownHash_LockHeld((PyObject *)od_copy, key, value,
_odictnode_HASH(node)) != 0)
int res = _PyODict_SetItem_KnownHash_LockHeld((PyObject *)od_copy,
key, value, hash);
Py_DECREF(key);
if (res != 0)
goto fail;
if (_PyODictObject_CAST(od)->od_state != state)
goto mutated;
}
}
else {
_odict_FOREACH(od, node) {
int res;
PyObject *value = PyObject_GetItem((PyObject *)od,
_odictnode_KEY(node));
if (value == NULL)
PyObject *key = Py_NewRef(_odictnode_KEY(node));
PyObject *value = PyObject_GetItem(od, key);
if (value == NULL) {
Py_DECREF(key);
goto fail;
res = PyObject_SetItem((PyObject *)od_copy,
_odictnode_KEY(node), value);
}
int res = PyObject_SetItem((PyObject *)od_copy, key, value);
Py_DECREF(value);
Py_DECREF(key);
if (res != 0)
goto fail;
if (_PyODictObject_CAST(od)->od_state != state)
goto mutated;
}
}
return od_copy;

mutated:
PyErr_SetString(PyExc_RuntimeError,
"OrderedDict mutated during iteration");
fail:
Py_DECREF(od_copy);
return NULL;
Expand Down
Loading