GH-92678: Make sure that tp_dictoffset is correct when Py_TPFLAGS_MANAGED_DICT is set

Lib/test/test_capi.py

@@ -538,6 +538,30 @@ def test_heaptype_with_dict(self):
         inst = _testcapi.HeapCTypeWithDict()
         self.assertEqual({}, inst.__dict__)
 
+    def test_heaptype_with_managed_dict(self):
+        inst = _testcapi.HeapCTypeWithManagedDict()
+        inst.foo = 42
+        self.assertEqual(inst.foo, 42)
+        self.assertEqual(inst.__dict__, {"foo": 42})
+
+        inst = _testcapi.HeapCTypeWithManagedDict()
+        self.assertEqual({}, inst.__dict__)
+
+        a = _testcapi.HeapCTypeWithManagedDict()
+        b = _testcapi.HeapCTypeWithManagedDict()
+        a.b = b
+        b.a = a
+        del a, b
+
+    def test_sublclassing_managed_dict(self):
+
+        class C(_testcapi.HeapCTypeWithManagedDict):
+            pass
+
+        i = C()
+        i.spam = i
+        del i
+
     def test_heaptype_with_negative_dict(self):
         inst = _testcapi.HeapCTypeWithNegativeDict()
         inst.foo = 42

Misc/NEWS.d/next/C API/2022-08-03-13-01-57.gh-issue-92678.DLwONN.rst

@@ -0,0 +1,2 @@
+Support C extensions using managed dictionaries by setting the
+``Py_TPFLAGS_MANAGED_DICT`` flag.

Modules/_testcapi/heaptype.c

@@ -745,6 +745,45 @@ static PyType_Spec HeapCTypeWithDict2_spec = {
     HeapCTypeWithDict_slots
 };
 
+static int
+heapmanaged_traverse(HeapCTypeObject *self, visitproc visit, void *arg)
+{
+    Py_VISIT(Py_TYPE(self));
+    return _PyObject_VisitManagedDict((PyObject *)self, visit, arg);
+}
+
+static void
+heapmanaged_clear(HeapCTypeObject *self)
+{
+    _PyObject_ClearManagedDict((PyObject *)self);
+}
+
+static void
+heapmanaged_dealloc(HeapCTypeObject *self)
+{
+    PyTypeObject *tp = Py_TYPE(self);
+    _PyObject_ClearManagedDict((PyObject *)self);
+    PyObject_GC_UnTrack(self);
+    PyObject_GC_Del(self);
+    Py_DECREF(tp);
+}
+
+static PyType_Slot HeapCTypeWithManagedDict_slots[] = {
+    {Py_tp_traverse, heapmanaged_traverse},
+    {Py_tp_getset, heapctypewithdict_getsetlist},
+    {Py_tp_clear, heapmanaged_clear},
+    {Py_tp_dealloc, heapmanaged_dealloc},
+    {0, 0},
+};
+
+static PyType_Spec  HeapCTypeWithManagedDict_spec = {
+    "_testcapi.HeapCTypeWithManagedDict",
+    sizeof(PyObject),
+    0,
+    Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_MANAGED_DICT,
+    HeapCTypeWithManagedDict_slots
+};
+
 static struct PyMemberDef heapctypewithnegativedict_members[] = {
     {"dictobj", T_OBJECT, offsetof(HeapCTypeWithDictObject, dict)},
     {"__dictoffset__", T_PYSSIZET, -(Py_ssize_t)sizeof(void*), READONLY},
@@ -947,6 +986,12 @@ _PyTestCapi_Init_Heaptype(PyObject *m) {
     }
     PyModule_AddObject(m, "HeapCTypeWithNegativeDict", HeapCTypeWithNegativeDict);
 
+    PyObject *HeapCTypeWithManagedDict = PyType_FromSpec(&HeapCTypeWithManagedDict_spec);
+    if (HeapCTypeWithManagedDict == NULL) {
+        return -1;
+    }
+    PyModule_AddObject(m, "HeapCTypeWithManagedDict", HeapCTypeWithManagedDict);
+
     PyObject *HeapCTypeWithWeakref = PyType_FromSpec(&HeapCTypeWithWeakref_spec);
     if (HeapCTypeWithWeakref == NULL) {
         return -1;

Objects/typeobject.c

@@ -1310,9 +1310,11 @@ subtype_traverse(PyObject *self, visitproc visit, void *arg)
     }
 
     if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) {
-        int err = _PyObject_VisitManagedDict(self, visit, arg);
-        if (err) {
-            return err;
+        if ((base->tp_flags & Py_TPFLAGS_MANAGED_DICT) == 0) {
+            int err = _PyObject_VisitManagedDict(self, visit, arg);
+            if (err) {
+                return err;
+            }
         }
     }
     else if (type->tp_dictoffset != base->tp_dictoffset) {
@@ -1377,7 +1379,9 @@ subtype_clear(PyObject *self)
     /* Clear the instance dict (if any), to break cycles involving only
        __dict__ slots (as in the case 'self.__dict__ is self'). */
     if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) {
-        _PyObject_ClearManagedDict(self);
+        if ((base->tp_flags & Py_TPFLAGS_MANAGED_DICT) == 0) {
+            _PyObject_ClearManagedDict(self);
+        }
     }
     else if (type->tp_dictoffset != base->tp_dictoffset) {
         PyObject **dictptr = _PyObject_ComputedDictPointer(self);
@@ -3091,7 +3095,7 @@ type_new_descriptors(const type_new_ctx *ctx, PyTypeObject *type)
     if (ctx->add_dict && ctx->base->tp_itemsize == 0) {
         assert((type->tp_flags & Py_TPFLAGS_MANAGED_DICT) == 0);
         type->tp_flags |= Py_TPFLAGS_MANAGED_DICT;
-        type->tp_dictoffset = -slotoffset - sizeof(PyObject *)*3;
+        type->tp_dictoffset = -slotoffset - (Py_ssize_t)sizeof(PyObject *)*3;
     }
 
     type->tp_basicsize = slotoffset;
@@ -6091,6 +6095,7 @@ inherit_special(PyTypeObject *type, PyTypeObject *base)
     COPYVAL(tp_itemsize);
     COPYVAL(tp_weaklistoffset);
     COPYVAL(tp_dictoffset);
+
 #undef COPYVAL
 
     /* Setup fast subclass flags */
@@ -6499,6 +6504,22 @@ type_ready_fill_dict(PyTypeObject *type)
     return 0;
 }
 
+static int
+type_ready_dict_offset(PyTypeObject *type)
+{
+    if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) {
+        if (type->tp_dictoffset > 0) {
+            PyErr_Format(PyExc_TypeError,
+                        "type %s has the Py_TPFLAGS_MANAGED_DICT flag "
+                        "but tp_dictoffset is positive",
+                        type->tp_name);
+            return -1;
+        }
+        Py_ssize_t offset = -type->tp_basicsize - sizeof(PyObject *)*3;
+        type->tp_dictoffset = offset;
+    }
+    return 0;
+}
 
 static int
 type_ready_mro(PyTypeObject *type)
@@ -6707,6 +6728,24 @@ type_ready_post_checks(PyTypeObject *type)
                      type->tp_name);
         return -1;
     }
+    if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) {
+        if (type->tp_itemsize != 0) {
+            PyErr_Format(PyExc_SystemError,
+                        "type %s has the Py_TPFLAGS_MANAGED_DICT flag "
+                        "but is variable sized",
+                        type->tp_name);
+            return -1;
+        }
+        Py_ssize_t size = type->tp_basicsize;
+        if (type->tp_dictoffset != -size - (Py_ssize_t)sizeof(PyObject *)*3) {
+            PyErr_Format(PyExc_SystemError,
+                        "type %s has the Py_TPFLAGS_MANAGED_DICT flag "
+                        "but tp_dictoffset is set to incompatible value",
+                        type->tp_name);
+            assert(0);
+            return -1;
+        }
+    }
     return 0;
 }
 
@@ -6746,6 +6785,9 @@ type_ready(PyTypeObject *type)
     if (type_ready_inherit(type) < 0) {
         return -1;
     }
+    if (type_ready_dict_offset(type) < 0) {
+        return -1;
+    }
     if (type_ready_set_hash(type) < 0) {
         return -1;
     }