From 8d8a08c7819cb48deb6c1a6522ce0b3c6a080847 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Fri, 26 Jun 2026 11:35:50 +0300 Subject: [PATCH 1/4] gh-152254: Fix a crash in `PyObject_ClearManagedDict` under memory pressure --- .../2026-06-26-11-34-50.gh-issue-152254.yK6TGp.rst | 2 ++ Objects/dictobject.c | 1 + 2 files changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-06-26-11-34-50.gh-issue-152254.yK6TGp.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-06-26-11-34-50.gh-issue-152254.yK6TGp.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-06-26-11-34-50.gh-issue-152254.yK6TGp.rst new file mode 100644 index 00000000000000..206b88a2affb2b --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-06-26-11-34-50.gh-issue-152254.yK6TGp.rst @@ -0,0 +1,2 @@ +Fix a crash during the clean up of managed dict objects under a memory +pressure in :c:func:`PyObject_ClearManagedDict`. diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 4b1a7bfe39d3a5..434cad0d12c2d7 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -7986,6 +7986,7 @@ PyObject_ClearManagedDict(PyObject *obj) /* Clear the dict */ Py_BEGIN_CRITICAL_SECTION(dict); PyDictKeysObject *oldkeys = dict->ma_keys; + ensure_shared_on_resize(dict); set_keys(dict, Py_EMPTY_KEYS); dict->ma_values = NULL; dictkeys_decref(oldkeys, IS_DICT_SHARED(dict)); From d2e2e463135d9e3de3260fe1b8eef6fcf67c4ee1 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Sat, 14 Nov 2026 10:12:49 +0300 Subject: [PATCH 2/4] Add tests --- Lib/test/test_capi/test_object.py | 74 +++++++++++++++++++++++++++++++ Modules/_testcapi/object.c | 8 ++++ 2 files changed, 82 insertions(+) diff --git a/Lib/test/test_capi/test_object.py b/Lib/test/test_capi/test_object.py index 338fc61ea1eb5f..dda4c8246c131e 100644 --- a/Lib/test/test_capi/test_object.py +++ b/Lib/test/test_capi/test_object.py @@ -354,6 +354,80 @@ def test_pyobject_dump(self): output = self.pyobject_dump(NULL) self.assertRegex(output, r'') + def test_object_clear_managed_dict(self): + # PyObject_ClearManagedDict + clear = _testcapi.pyobject_clear_managed_dict + ManagedDictType = _testcapi.ManagedDictType + + obj = ManagedDictType() + obj.value = 'first' + # asserting `__dict__` is a must to materialize the value before clean: + self.assertEqual(obj.__dict__, {'value': 'first'}) + self.assertIs(type(obj.__dict__), dict) + clear(obj) + self.assertEqual(obj.__dict__, {}) + self.assertIs(type(obj.__dict__), dict) + + # big object: + obj = ManagedDictType() + existing_dict = {f'attr_{x}': x for x in range(100)} + obj.__dict__ = existing_dict + self.assertIs(obj.__dict__, existing_dict) + clear(obj) + self.assertEqual(obj.__dict__, {}) + self.assertIs(type(obj.__dict__), dict) + + # empty dict: + obj = ManagedDictType() + self.assertEqual(obj.__dict__, {}) + self.assertIs(type(obj.__dict__), dict) + clear(obj) + self.assertEqual(obj.__dict__, {}) + self.assertIs(type(obj.__dict__), dict) + + # double clear: + obj = ManagedDictType() + obj.value = 'first' + self.assertEqual(obj.__dict__, {'value': 'first'}) + clear(obj) + clear(obj) + self.assertEqual(obj.__dict__, {}) + self.assertIs(type(obj.__dict__), dict) + + # unmaterialized: + obj = ManagedDictType() + obj.value = 'first' + clear(obj) + self.assertEqual(obj.__dict__, {}) + self.assertIs(type(obj.__dict__), dict) + + # CRASHES: clear(NULL) + + @threading_helper.requires_working_threading() + def test_object_clear_managed_dict_ft_memory_error(self): + # PyObject_ClearManagedDict, gh-152254 + clear = _testcapi.pyobject_clear_managed_dict + ManagedDictType = _testcapi.ManagedDictType + + obj = ManagedDictType() + obj.value = 'first' + self.assertEqual(len(obj.__dict__), 1) + + def worker(): + # This needs to be in a separate thread for the crash to happen: + with support.catch_unraisable_exception() as ex: + # `clear` used to crash: + _testcapi.call_with_nomemory(0, 1, clear, obj) + self.assertIsNotNone(ex.unraisable) + self.assertIs(ex.unraisable.exc_type, MemoryError) + self.assertIn('managed dict', ex.unraisable.err_msg) + self.assertEqual(obj.__dict__, {}) + self.assertIs(type(obj.__dict__), dict) + + threading_helper.run_concurrently([worker]) + self.assertEqual(obj.__dict__, {}) + self.assertIs(type(obj.__dict__), dict) + class RefTracerTest(unittest.TestCase): @support.requires_resource('cpu') diff --git a/Modules/_testcapi/object.c b/Modules/_testcapi/object.c index 8fcb86e33e55a8..1bb038aea654a1 100644 --- a/Modules/_testcapi/object.c +++ b/Modules/_testcapi/object.c @@ -153,6 +153,13 @@ pyobject_is_unique_temporary_new_object(PyObject *self, PyObject *unused) return PyLong_FromLong(result); } +static PyObject * +pyobject_clear_managed_dict(PyObject *self, PyObject *obj) +{ + PyObject_ClearManagedDict(obj); + Py_RETURN_NONE; +} + static int MyObject_dealloc_called = 0; static void @@ -358,6 +365,7 @@ static PyMethodDef test_methods[] = { {"pyobject_enable_deferred_refcount", pyobject_enable_deferred_refcount, METH_O}, {"pyobject_is_unique_temporary", pyobject_is_unique_temporary, METH_O}, {"pyobject_is_unique_temporary_new_object", pyobject_is_unique_temporary_new_object, METH_NOARGS}, + {"pyobject_clear_managed_dict", pyobject_clear_managed_dict, METH_O}, {"test_py_try_inc_ref", test_py_try_inc_ref, METH_NOARGS}, {"test_py_set_immortal", test_py_set_immortal, METH_NOARGS}, #ifdef Py_REF_DEBUG From 0c260e13d1554bcecfa7633f86077cff4b8d3df0 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Sun, 15 Nov 2026 11:30:25 +0300 Subject: [PATCH 3/4] Address review --- Lib/test/test_capi/test_object.py | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/Lib/test/test_capi/test_object.py b/Lib/test/test_capi/test_object.py index dda4c8246c131e..e54825f6d6f08b 100644 --- a/Lib/test/test_capi/test_object.py +++ b/Lib/test/test_capi/test_object.py @@ -356,7 +356,7 @@ def test_pyobject_dump(self): def test_object_clear_managed_dict(self): # PyObject_ClearManagedDict - clear = _testcapi.pyobject_clear_managed_dict + clear_managed_dict = _testcapi.pyobject_clear_managed_dict ManagedDictType = _testcapi.ManagedDictType obj = ManagedDictType() @@ -364,7 +364,7 @@ def test_object_clear_managed_dict(self): # asserting `__dict__` is a must to materialize the value before clean: self.assertEqual(obj.__dict__, {'value': 'first'}) self.assertIs(type(obj.__dict__), dict) - clear(obj) + clear_managed_dict(obj) self.assertEqual(obj.__dict__, {}) self.assertIs(type(obj.__dict__), dict) @@ -373,7 +373,7 @@ def test_object_clear_managed_dict(self): existing_dict = {f'attr_{x}': x for x in range(100)} obj.__dict__ = existing_dict self.assertIs(obj.__dict__, existing_dict) - clear(obj) + clear_managed_dict(obj) self.assertEqual(obj.__dict__, {}) self.assertIs(type(obj.__dict__), dict) @@ -381,7 +381,7 @@ def test_object_clear_managed_dict(self): obj = ManagedDictType() self.assertEqual(obj.__dict__, {}) self.assertIs(type(obj.__dict__), dict) - clear(obj) + clear_managed_dict(obj) self.assertEqual(obj.__dict__, {}) self.assertIs(type(obj.__dict__), dict) @@ -389,24 +389,27 @@ def test_object_clear_managed_dict(self): obj = ManagedDictType() obj.value = 'first' self.assertEqual(obj.__dict__, {'value': 'first'}) - clear(obj) - clear(obj) + clear_managed_dict(obj) + clear_managed_dict(obj) self.assertEqual(obj.__dict__, {}) self.assertIs(type(obj.__dict__), dict) # unmaterialized: obj = ManagedDictType() obj.value = 'first' - clear(obj) + clear_managed_dict(obj) self.assertEqual(obj.__dict__, {}) self.assertIs(type(obj.__dict__), dict) - # CRASHES: clear(NULL) + # CRASHES: clear_managed_dict(NULL) @threading_helper.requires_working_threading() + @support.nomemtest def test_object_clear_managed_dict_ft_memory_error(self): # PyObject_ClearManagedDict, gh-152254 - clear = _testcapi.pyobject_clear_managed_dict + # The setup must contain: object owned by another thread + # and the attempt to clear the managed dict with no memory left + clear_managed_dict = _testcapi.pyobject_clear_managed_dict ManagedDictType = _testcapi.ManagedDictType obj = ManagedDictType() @@ -416,8 +419,8 @@ def test_object_clear_managed_dict_ft_memory_error(self): def worker(): # This needs to be in a separate thread for the crash to happen: with support.catch_unraisable_exception() as ex: - # `clear` used to crash: - _testcapi.call_with_nomemory(0, 1, clear, obj) + # `clear_managed_dict` used to crash: + _testcapi.call_with_nomemory(0, 1, clear_managed_dict, obj) self.assertIsNotNone(ex.unraisable) self.assertIs(ex.unraisable.exc_type, MemoryError) self.assertIn('managed dict', ex.unraisable.err_msg) From 92296a1d88b97fffc39e68c0f22dd6b0ca523bae Mon Sep 17 00:00:00 2001 From: sobolevn Date: Sun, 15 Nov 2026 15:03:26 +0300 Subject: [PATCH 4/4] Apply batched suggestions from code review Co-authored-by: Victor Stinner --- Lib/test/test_capi/test_object.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_capi/test_object.py b/Lib/test/test_capi/test_object.py index e54825f6d6f08b..ece673dbbec089 100644 --- a/Lib/test/test_capi/test_object.py +++ b/Lib/test/test_capi/test_object.py @@ -406,9 +406,9 @@ def test_object_clear_managed_dict(self): @threading_helper.requires_working_threading() @support.nomemtest def test_object_clear_managed_dict_ft_memory_error(self): - # PyObject_ClearManagedDict, gh-152254 + # gh-152254: Test PyObject_ClearManagedDict() error. # The setup must contain: object owned by another thread - # and the attempt to clear the managed dict with no memory left + # and the attempt to clear the managed dict with no memory left. clear_managed_dict = _testcapi.pyobject_clear_managed_dict ManagedDictType = _testcapi.ManagedDictType @@ -419,7 +419,7 @@ def test_object_clear_managed_dict_ft_memory_error(self): def worker(): # This needs to be in a separate thread for the crash to happen: with support.catch_unraisable_exception() as ex: - # `clear_managed_dict` used to crash: + # `PyObject_ClearManagedDict()` used to crash: _testcapi.call_with_nomemory(0, 1, clear_managed_dict, obj) self.assertIsNotNone(ex.unraisable) self.assertIs(ex.unraisable.exc_type, MemoryError)