diff --git a/Lib/test/test_asyncio/test_ssl.py b/Lib/test/test_asyncio/test_ssl.py index ca15fc3bdd42dd..784c784d261dc6 100644 --- a/Lib/test/test_asyncio/test_ssl.py +++ b/Lib/test/test_asyncio/test_ssl.py @@ -1544,6 +1544,9 @@ async def client(addr): # This triggers bug gh-115514, also tested using mocks in # test.test_asyncio.test_selector_events.SelectorSocketTransportTests.test_write_buffer_after_close socket_transport = writer.transport._ssl_protocol._transport + # connection_lost may have already cleared _transport. + if socket_transport is None: + return class SocketWrapper: def __init__(self, sock) -> None: diff --git a/Lib/test/test_ordered_dict.py b/Lib/test/test_ordered_dict.py index 642c2722711c7c..a1b1878bcfbac5 100644 --- a/Lib/test/test_ordered_dict.py +++ b/Lib/test/test_ordered_dict.py @@ -879,6 +879,149 @@ def side_effect(self): self.assertDictEqual(dict1, dict.fromkeys((0, 4.2))) self.assertDictEqual(dict2, dict.fromkeys((0, Key(), 4.2))) + def check_copy_runtime_error_issue148660(self, od): + msg = re.escape("OrderedDict mutated during iteration") + self.assertRaisesRegex(RuntimeError, msg, od.copy) + self.assertEqual(len(od), 0) # the side effect cleared it + + def test_issue148660_copy_clear_in_source_lookup(self): + # gh-148660: a key's __eq__ clears od while od.copy() looks the value + # up in the source dict; the loop must not read the freed nodes. + OrderedDict = self.OrderedDict + armed = False + + class Key: + def __hash__(self): + return 42 # force collisions so __eq__ runs during copy + def __eq__(self, other): + if armed: + od.clear() + return self is other + + od = OrderedDict([(Key(), 'v1'), (Key(), 'v2')]) + armed = True + self.check_copy_runtime_error_issue148660(od) + + def test_issue148660_copy_clear_in_dest_insert(self): + # gh-148660: this is the ASan-reported path -- the key's __eq__ clears + # od while od.copy() inserts into the *destination* dict (the collision + # probe into od_copy), not during the source lookup. + OrderedDict = self.OrderedDict + armed = False + eq_calls = 0 + + class Key: + def __hash__(self): + return 42 # force collisions so __eq__ runs during copy + def __eq__(self, other): + nonlocal eq_calls + if armed: + eq_calls += 1 + # 1st armed __eq__ is the source lookup, 2nd is the + # collision probe while inserting into od_copy. + if eq_calls == 2: + od.clear() + return self is other + + od = OrderedDict([(Key(), 'v1'), (Key(), 'v2')]) + armed = True + self.check_copy_runtime_error_issue148660(od) + + def test_issue148660_copy_clear_in_hash(self): + # gh-148660: a key's __hash__ clears od while od.copy() re-hashes it. + OrderedDict = self.OrderedDict + armed = False + + class Key: + def __init__(self, n): + self.n = n + def __hash__(self): + if armed: + od.clear() + return 42 + def __eq__(self, other): + return self is other + + od = OrderedDict([(Key(1), 'v1'), (Key(2), 'v2')]) + armed = True + self.check_copy_runtime_error_issue148660(od) + + def test_issue148660_copy_key_eq_exception_is_preserved(self): + # gh-148660: if a key's __eq__ both mutates od and raises, the raised + # exception is preserved, matching OrderedDict.__eq__ (gh-119004). + OrderedDict = self.OrderedDict + armed = False + + class Boom(Exception): + pass + + class Key: + def __hash__(self): + return 42 + def __eq__(self, other): + if armed: + od.clear() + raise Boom + return self is other + + od = OrderedDict([(Key(), 'v1'), (Key(), 'v2')]) + armed = True + self.assertRaises(Boom, od.copy) + + def test_issue148660_copy_clear_in_subclass_getitem(self): + # gh-148660: a subclass __getitem__ clears od during od.copy(). + OrderedDict = self.OrderedDict + armed = False + + class OD(OrderedDict): + def __getitem__(self, key): + if armed: + od.clear() + return 'dummy' + return OrderedDict.__getitem__(self, key) + + od = OD([(1, 'v1'), (2, 'v2')]) + armed = True + self.check_copy_runtime_error_issue148660(od) + + def test_issue148660_copy_clear_in_subclass_setitem(self): + # gh-148660: a subclass __setitem__ clears od during od.copy(). + OrderedDict = self.OrderedDict + armed = False + + class OD(OrderedDict): + def __setitem__(self, key, value): + if armed: + od.clear() + OrderedDict.__setitem__(self, key, value) + + od = OD([(1, 'v1'), (2, 'v2')]) + armed = True + self.check_copy_runtime_error_issue148660(od) + + def test_issue148660_copy_clear_in_value_del(self): + # gh-148660: a value's __del__ (run by Py_DECREF) clears od during + # od.copy(). + OrderedDict = self.OrderedDict + armed = False + + class V: + def __del__(self): + if armed: + od.clear() + + class OD(OrderedDict): + def __getitem__(self, key): + return V() + def __setitem__(self, key, value): + pass + + od = OD() + OrderedDict.__setitem__(od, 1, 'v1') + OrderedDict.__setitem__(od, 2, 'v2') + armed = True + self.check_copy_runtime_error_issue148660(od) + @unittest.skipUnless(c_coll, 'requires the C version of the collections module') class CPythonOrderedDictTests(OrderedDictTests, diff --git a/Misc/NEWS.d/next/Library/2026-06-16-04-27-07.gh-issue-148660.QUbmVk.rst b/Misc/NEWS.d/next/Library/2026-06-16-04-27-07.gh-issue-148660.QUbmVk.rst new file mode 100644 index 00000000000000..8a04711b061306 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-06-16-04-27-07.gh-issue-148660.QUbmVk.rst @@ -0,0 +1,5 @@ +Fix a crash in :meth:`collections.OrderedDict.copy` when the dict is mutated +while being copied, for example by a key's ``__eq__`` or ``__hash__``, a +subclass' ``__getitem__`` or ``__setitem__``, or a value's ``__del__``. It +now raises :exc:`RuntimeError`, like other operations that mutate an +:class:`~collections.OrderedDict` during iteration. Patch by Harjoth Khara. diff --git a/Objects/odictobject.c b/Objects/odictobject.c index 14c1b02405822c..beb0a3dfa57e22 100644 --- a/Objects/odictobject.c +++ b/Objects/odictobject.c @@ -1251,32 +1251,92 @@ OrderedDict_copy_impl(PyObject *od) if (od_copy == NULL) return NULL; + /* Building the copy runs arbitrary Python code: a key's __hash__/__eq__, + a subclass' __getitem__/__setitem__, or a value's __del__ may mutate od + and free the nodes we are iterating over. Keep od's state to detect + such mutations and raise instead of dereferencing freed memory + (see gh-148660), mirroring what is already done when comparing + OrderedDicts (see gh-119004). */ + const 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); + node = _odict_FIRST(od); + while (node != NULL) { + PyObject *key = Py_NewRef(_odictnode_KEY(node)); + Py_hash_t hash = _odictnode_HASH(node); + /* The value lookup may run the key's __eq__ or __hash__. */ + PyObject *value = PyODict_GetItemWithError((PyObject *)od, key); + /* Propagate an exception raised by that re-entrant call before + reporting a mutation, as OrderedDict.__eq__ does (gh-119004). */ + if (value == NULL && PyErr_Occurred()) { + Py_DECREF(key); + goto fail; + } + if (_PyODictObject_CAST(od)->od_state != state) { + Py_DECREF(key); + PyErr_SetString(PyExc_RuntimeError, + "OrderedDict mutated during iteration"); + goto fail; + } if (value == NULL) { - if (!PyErr_Occurred()) - PyErr_SetObject(PyExc_KeyError, key); + PyErr_SetObject(PyExc_KeyError, key); + Py_DECREF(key); goto fail; } - if (_PyODict_SetItem_KnownHash_LockHeld((PyObject *)od_copy, key, value, - _odictnode_HASH(node)) != 0) + /* value is borrowed, but the insert refs it before running any + __eq__ of its own, so it cannot be freed under us here. */ + int res = _PyODict_SetItem_KnownHash_LockHeld((PyObject *)od_copy, + key, value, hash); + Py_DECREF(key); + if (res != 0) + goto fail; + /* The destination insert may run the key's __eq__ and mutate od. */ + if (_PyODictObject_CAST(od)->od_state != state) { + PyErr_SetString(PyExc_RuntimeError, + "OrderedDict mutated during iteration"); goto fail; + } + node = _odictnode_NEXT(node); } } else { - _odict_FOREACH(od, node) { - int res; - PyObject *value = PyObject_GetItem((PyObject *)od, - _odictnode_KEY(node)); - if (value == NULL) + node = _odict_FIRST(od); + while (node != NULL) { + PyObject *key = Py_NewRef(_odictnode_KEY(node)); + /* __getitem__ (or the key's __eq__ during the lookup) may run + Python code that mutates od. */ + PyObject *value = PyObject_GetItem((PyObject *)od, key); + if (value == NULL) { + /* A re-entrant mutation that drops the key makes __getitem__ + raise KeyError; report the mutation instead, but let any + other exception propagate (as OrderedDict.__eq__ does). */ + if (_PyODictObject_CAST(od)->od_state != state + && PyErr_ExceptionMatches(PyExc_KeyError)) { + PyErr_SetString(PyExc_RuntimeError, + "OrderedDict mutated during iteration"); + } + Py_DECREF(key); + goto fail; + } + if (_PyODictObject_CAST(od)->od_state != state) { + Py_DECREF(value); + Py_DECREF(key); + PyErr_SetString(PyExc_RuntimeError, + "OrderedDict mutated during iteration"); 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; + /* __setitem__ on the copy, or a value's __del__, may mutate od. */ + if (_PyODictObject_CAST(od)->od_state != state) { + PyErr_SetString(PyExc_RuntimeError, + "OrderedDict mutated during iteration"); + goto fail; + } + node = _odictnode_NEXT(node); } } return od_copy;