From 0eb461661144ed84e46fbdc3ae71701c9a2a9c0b Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 28 May 2025 19:03:41 +0100 Subject: [PATCH] GH-133912: Fix `PyObject_GenericSetDict` to handle inline values (GH-134725) (cherry picked from commit 9fbd66a93d526c49fac8e1427c25e8f7f4154e29) Co-authored-by: Mark Shannon --- Include/internal/pycore_object.h | 2 + Lib/test/test_capi/test_type.py | 10 ++++ ...-05-26-15-55-50.gh-issue-133912.-xAguL.rst | 2 + Modules/_testcapimodule.c | 49 +++++++++++++++++ Objects/object.c | 31 +---------- Objects/typeobject.c | 54 +++++++++++-------- 6 files changed, 95 insertions(+), 53 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-05-26-15-55-50.gh-issue-133912.-xAguL.rst diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index b7e162c8abcabf..942de9de251029 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -1007,6 +1007,8 @@ enum _PyAnnotateFormat { _Py_ANNOTATE_FORMAT_STRING = 4, }; +int _PyObject_SetDict(PyObject *obj, PyObject *value); + #ifdef __cplusplus } #endif diff --git a/Lib/test/test_capi/test_type.py b/Lib/test/test_capi/test_type.py index 3c9974c7387388..15fb4a93e2ad74 100644 --- a/Lib/test/test_capi/test_type.py +++ b/Lib/test/test_capi/test_type.py @@ -264,3 +264,13 @@ def test_manual_heap_type(self): ManualHeapType = _testcapi.ManualHeapType for i in range(100): self.assertIsInstance(ManualHeapType(), ManualHeapType) + + def test_extension_managed_dict_type(self): + ManagedDictType = _testcapi.ManagedDictType + obj = ManagedDictType() + obj.foo = 42 + self.assertEqual(obj.foo, 42) + self.assertEqual(obj.__dict__, {'foo': 42}) + obj.__dict__ = {'bar': 3} + self.assertEqual(obj.__dict__, {'bar': 3}) + self.assertEqual(obj.bar, 3) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-05-26-15-55-50.gh-issue-133912.-xAguL.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-05-26-15-55-50.gh-issue-133912.-xAguL.rst new file mode 100644 index 00000000000000..2118f3d0c350ec --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-05-26-15-55-50.gh-issue-133912.-xAguL.rst @@ -0,0 +1,2 @@ +Fix the C API function ``PyObject_GenericSetDict`` to handle extension +classes with inline values. diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 3aa6e4c9e43a26..281c5b41137ac2 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -3175,6 +3175,48 @@ create_manual_heap_type(void) return (PyObject *)type; } +typedef struct { + PyObject_VAR_HEAD +} ManagedDictObject; + +int ManagedDict_traverse(PyObject *self, visitproc visit, void *arg) { + PyObject_VisitManagedDict(self, visit, arg); + Py_VISIT(Py_TYPE(self)); + return 0; +} + +int ManagedDict_clear(PyObject *self) { + PyObject_ClearManagedDict(self); + return 0; +} + +static PyGetSetDef ManagedDict_getset[] = { + {"__dict__", PyObject_GenericGetDict, PyObject_GenericSetDict, NULL, NULL}, + {NULL, NULL, NULL, NULL, NULL}, +}; + +static PyType_Slot ManagedDict_slots[] = { + {Py_tp_new, (void *)PyType_GenericNew}, + {Py_tp_getset, (void *)ManagedDict_getset}, + {Py_tp_traverse, (void *)ManagedDict_traverse}, + {Py_tp_clear, (void *)ManagedDict_clear}, + {0} +}; + +static PyType_Spec ManagedDict_spec = { + "_testcapi.ManagedDictType", + sizeof(ManagedDictObject), + 0, // itemsize + Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_MANAGED_DICT | Py_TPFLAGS_HEAPTYPE | Py_TPFLAGS_HAVE_GC, + ManagedDict_slots +}; + +static PyObject * +create_managed_dict_type(void) +{ + return PyType_FromSpec(&ManagedDict_spec); +} + static struct PyModuleDef _testcapimodule = { PyModuleDef_HEAD_INIT, .m_name = "_testcapi", @@ -3315,6 +3357,13 @@ PyInit__testcapi(void) return NULL; } + PyObject *managed_dict_type = create_managed_dict_type(); + if (managed_dict_type == NULL) { + return NULL; + } + if (PyModule_Add(m, "ManagedDictType", managed_dict_type) < 0) { + return NULL; + } /* Include tests from the _testcapi/ directory */ if (_PyTestCapi_Init_Vectorcall(m) < 0) { diff --git a/Objects/object.c b/Objects/object.c index af1aa217f75462..5414b66b0bd8a3 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1906,40 +1906,11 @@ PyObject_GenericSetAttr(PyObject *obj, PyObject *name, PyObject *value) int PyObject_GenericSetDict(PyObject *obj, PyObject *value, void *context) { - PyObject **dictptr = _PyObject_GetDictPtr(obj); - if (dictptr == NULL) { - if (_PyType_HasFeature(Py_TYPE(obj), Py_TPFLAGS_INLINE_VALUES) && - _PyObject_GetManagedDict(obj) == NULL - ) { - /* Was unable to convert to dict */ - PyErr_NoMemory(); - } - else { - PyErr_SetString(PyExc_AttributeError, - "This object has no __dict__"); - } - return -1; - } if (value == NULL) { PyErr_SetString(PyExc_TypeError, "cannot delete __dict__"); return -1; } - if (!PyDict_Check(value)) { - PyErr_Format(PyExc_TypeError, - "__dict__ must be set to a dictionary, " - "not a '%.200s'", Py_TYPE(value)->tp_name); - return -1; - } - Py_BEGIN_CRITICAL_SECTION(obj); - PyObject *olddict = *dictptr; - FT_ATOMIC_STORE_PTR_RELEASE(*dictptr, Py_NewRef(value)); -#ifdef Py_GIL_DISABLED - _PyObject_XDecRefDelayed(olddict); -#else - Py_XDECREF(olddict); -#endif - Py_END_CRITICAL_SECTION(); - return 0; + return _PyObject_SetDict(obj, value); } diff --git a/Objects/typeobject.c b/Objects/typeobject.c index ee09289425b91a..32bbaecf9a3d87 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3676,10 +3676,39 @@ subtype_dict(PyObject *obj, void *context) return PyObject_GenericGetDict(obj, context); } +int +_PyObject_SetDict(PyObject *obj, PyObject *value) +{ + if (value != NULL && !PyDict_Check(value)) { + PyErr_Format(PyExc_TypeError, + "__dict__ must be set to a dictionary, " + "not a '%.200s'", Py_TYPE(value)->tp_name); + return -1; + } + if (Py_TYPE(obj)->tp_flags & Py_TPFLAGS_MANAGED_DICT) { + return _PyObject_SetManagedDict(obj, value); + } + PyObject **dictptr = _PyObject_ComputedDictPointer(obj); + if (dictptr == NULL) { + PyErr_SetString(PyExc_AttributeError, + "This object has no __dict__"); + return -1; + } + Py_BEGIN_CRITICAL_SECTION(obj); + PyObject *olddict = *dictptr; + FT_ATOMIC_STORE_PTR_RELEASE(*dictptr, Py_NewRef(value)); +#ifdef Py_GIL_DISABLED + _PyObject_XDecRefDelayed(olddict); +#else + Py_XDECREF(olddict); +#endif + Py_END_CRITICAL_SECTION(); + return 0; +} + static int subtype_setdict(PyObject *obj, PyObject *value, void *context) { - PyObject **dictptr; PyTypeObject *base; base = get_builtin_base_with_dict(Py_TYPE(obj)); @@ -3697,28 +3726,7 @@ subtype_setdict(PyObject *obj, PyObject *value, void *context) } return func(descr, obj, value); } - /* Almost like PyObject_GenericSetDict, but allow __dict__ to be deleted. */ - if (value != NULL && !PyDict_Check(value)) { - PyErr_Format(PyExc_TypeError, - "__dict__ must be set to a dictionary, " - "not a '%.200s'", Py_TYPE(value)->tp_name); - return -1; - } - - if (Py_TYPE(obj)->tp_flags & Py_TPFLAGS_MANAGED_DICT) { - return _PyObject_SetManagedDict(obj, value); - } - else { - dictptr = _PyObject_ComputedDictPointer(obj); - if (dictptr == NULL) { - PyErr_SetString(PyExc_AttributeError, - "This object has no __dict__"); - return -1; - } - Py_CLEAR(*dictptr); - *dictptr = Py_XNewRef(value); - } - return 0; + return _PyObject_SetDict(obj, value); } static PyObject *