Revert last changes

Author: sergey-miryanov

Doc/c-api/typeobj.rst

@@ -1260,7 +1260,7 @@ and :c:data:`PyType_Type` effectively act as defaults.)
       This bit indicates that instances of the class have a :attr:`~object.__dict__`
       attribute, and that the space for the dictionary is managed by the VM.
 
-      If this flag is set, :c:macro:`Py_TPFLAGS_HAVE_GC` must also be set.
+      If this flag is set, :c:macro:`Py_TPFLAGS_HAVE_GC` should also be set.
 
       The type traverse function must call :c:func:`PyObject_VisitManagedDict`
       and its clear function must call :c:func:`PyObject_ClearManagedDict`.
@@ -1278,7 +1278,7 @@ and :c:data:`PyType_Type` effectively act as defaults.)
       This bit indicates that instances of the class should be weakly
       referenceable.
 
-      If this flag is set, :c:macro:`Py_TPFLAGS_HAVE_GC` must also be set.
+      If this flag is set, :c:macro:`Py_TPFLAGS_HAVE_GC` should also be set.
 
       .. versionadded:: 3.12
 

Doc/whatsnew/3.15.rst

@@ -785,6 +785,13 @@ New features
   (Contributed by Victor Stinner in :gh:`129813`.)
 
 
+Limited C API changes
+---------------------
+
+* If the :c:macro:`Py_TPFLAGS_MANAGED_DICT` and :c:macro:`Py_TPFLAGS_MANAGED_WEAKREF`
+  flags are set then :c:macro:`Py_TPFLAGS_HAVE_GC` should be set too.
+  (Contributed by Sergey Miryanov in :gh:`134786`)
+
 
 Porting to Python 3.15
 ----------------------

Include/internal/pycore_object.h

@@ -936,13 +936,9 @@ extern int _PyType_CacheInitForSpecialization(PyHeapTypeObject *type,
 #ifdef Py_GIL_DISABLED
 #  define MANAGED_DICT_OFFSET    (((Py_ssize_t)sizeof(PyObject *))*-1)
 #  define MANAGED_WEAKREF_OFFSET (((Py_ssize_t)sizeof(PyObject *))*-2)
-#  define MANAGED_DICT_OFFSET_NO_GC    (((Py_ssize_t)sizeof(PyObject *))*-1)
-#  define MANAGED_WEAKREF_OFFSET_NO_GC (((Py_ssize_t)sizeof(PyObject *))*-2)
 #else
 #  define MANAGED_DICT_OFFSET    (((Py_ssize_t)sizeof(PyObject *))*-3)
 #  define MANAGED_WEAKREF_OFFSET (((Py_ssize_t)sizeof(PyObject *))*-4)
-#  define MANAGED_DICT_OFFSET_NO_GC    (((Py_ssize_t)sizeof(PyObject *))*-1)
-#  define MANAGED_WEAKREF_OFFSET_NO_GC (((Py_ssize_t)sizeof(PyObject *))*-2)
 #endif
 
 typedef union {
@@ -952,13 +948,8 @@ typedef union {
 static inline PyManagedDictPointer *
 _PyObject_ManagedDictPointer(PyObject *obj)
 {
-    PyTypeObject *type = Py_TYPE(obj);
-    assert(type->tp_flags & Py_TPFLAGS_MANAGED_DICT);
-    Py_ssize_t offset = MANAGED_DICT_OFFSET;
-    if (!_PyType_IS_GC(type)) {
-        offset = MANAGED_DICT_OFFSET_NO_GC;
-    }
-    return (PyManagedDictPointer *)((char *)obj + offset);
+    assert(Py_TYPE(obj)->tp_flags & Py_TPFLAGS_MANAGED_DICT);
+    return (PyManagedDictPointer *)((char *)obj + MANAGED_DICT_OFFSET);
 }
 
 static inline PyDictObject *

Include/object.h

@@ -529,7 +529,7 @@ given type object has a specified feature.
 #define Py_TPFLAGS_INLINE_VALUES (1 << 2)
 
 /* Placement of weakref pointers are managed by the VM, not by the type.
- * The VM will automatically set tp_weaklistoffset.
+ * The VM will automatically set tp_weaklistoffset. Implies Py_TPFLAGS_HAVE_GC.
  */
 #define Py_TPFLAGS_MANAGED_WEAKREF (1 << 3)
 

Lib/test/test_capi/test_type.py

@@ -1,6 +1,5 @@
 from test.support import import_helper, Py_GIL_DISABLED, refleak_helper
 import unittest
-import weakref
 
 _testcapi = import_helper.import_module('_testcapi')
 
@@ -275,16 +274,3 @@ def test_extension_managed_dict_type(self):
         obj.__dict__ = {'bar': 3}
         self.assertEqual(obj.__dict__, {'bar': 3})
         self.assertEqual(obj.bar, 3)
-
-    def test_type_have_managed_weakref_and_no_gc(self):
-        ManagedWeakrefNoGCType = _testcapi.ManagedWeakrefNoGCType
-        obj = ManagedWeakrefNoGCType()
-        wr = weakref.ref(obj)
-
-        del obj # shouldn't segfault
-        del wr
-
-    def test_type_have_managed_dict_and_no_gc(self):
-        ManagedDictNoGCType = _testcapi.ManagedDictNoGCType
-        obj = ManagedDictNoGCType()
-        del obj # shouldn't segfault

Misc/NEWS.d/next/Core_and_Builtins/2025-06-24-13-12-58.gh-issue-134786.MF0VVk.rst

@@ -1,3 +1,3 @@
-Fix segmentation fault when deallocating object of the type with
-:c:macro:`Py_TPFLAGS_MANAGED_WEAKREF` or :c:macro:`Py_TPFLAGS_MANAGED_DICT`
-enabled but without :c:macro:`Py_TPFLAGS_HAVE_GC`.
+Force to use :c:macro:`Py_TPFLAGS_HAVE_GC` if
+:c:macro:`Py_TPFLAGS_MANAGED_DICT` or :c:macro:`Py_TPFLAGS_MANAGED_WEAKREF`
+used.

Modules/_testcapimodule.c

@@ -3227,74 +3227,12 @@ static PyType_Spec ManagedDict_spec = {
     ManagedDict_slots
 };
 
-typedef struct {
-    PyObject_HEAD
-} ManagedWeakrefNoGCObject;
-
-static void
-ManagedWeakrefNoGC_dealloc(PyObject *self)
-{
-    PyObject_ClearWeakRefs(self);
-    PyTypeObject *tp = Py_TYPE(self);
-    tp->tp_free(self);
-    Py_DECREF(tp);
-}
-
-static PyType_Slot ManagedWeakrefNoGC_slots[] = {
-    {Py_tp_dealloc, ManagedWeakrefNoGC_dealloc},
-    {0, 0}
-};
-
-static PyType_Spec ManagedWeakrefNoGC_spec = {
-    .name = "_testcapi.ManagedWeakrefNoGCType",
-    .basicsize = sizeof(ManagedWeakrefNoGCObject),
-    .flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_MANAGED_WEAKREF),
-    .slots = ManagedWeakrefNoGC_slots,
-};
-
-typedef struct {
-    PyObject_HEAD
-} ManagedDictNoGCObject;
-
-static void
-ManagedDictNoGC_dealloc(PyObject *self)
-{
-    PyTypeObject *tp = Py_TYPE(self);
-    tp->tp_free(self);
-    Py_DECREF(tp);
-}
-
-static PyType_Slot ManagedDictNoGC_slots[] = {
-    {Py_tp_dealloc, ManagedDictNoGC_dealloc},
-    {0, 0}
-};
-
-static PyType_Spec ManagedDictNoGC_spec = {
-    .name = "_testcapi.ManagedDictNoGCType",
-    .basicsize = sizeof(ManagedDictNoGCObject),
-    .flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_MANAGED_DICT),
-    .slots = ManagedDictNoGC_slots,
-};
-
-
 static PyObject *
 create_managed_dict_type(void)
 {
    return PyType_FromSpec(&ManagedDict_spec);
 }
 
-static PyObject *
-create_managed_weakref_no_gc_type(void)
-{
-   return PyType_FromSpec(&ManagedWeakrefNoGC_spec);
-}
-
-static PyObject *
-create_managed_dict_no_gc_type(void)
-{
-   return PyType_FromSpec(&ManagedDictNoGC_spec);
-}
-
 static int
 _testcapi_exec(PyObject *m)
 {
@@ -3424,22 +3362,6 @@ _testcapi_exec(PyObject *m)
         return -1;
     }
 
-    PyObject *managed_weakref_no_gc_type = create_managed_weakref_no_gc_type();
-    if (managed_weakref_no_gc_type == NULL) {
-        return -1;
-    }
-    if (PyModule_Add(m, "ManagedWeakrefNoGCType", managed_weakref_no_gc_type) < 0) {
-        return -1;
-    }
-
-    PyObject *managed_dict_no_gc_type = create_managed_dict_no_gc_type();
-    if (managed_dict_no_gc_type == NULL) {
-        return -1;
-    }
-    if (PyModule_Add(m, "ManagedDictNoGCType", managed_dict_no_gc_type) < 0) {
-        return -1;
-    }
-
     /* Include tests from the _testcapi/ directory */
     if (_PyTestCapi_Init_Vectorcall(m) < 0) {
         return -1;

Objects/typeobject.c

@@ -4655,12 +4655,7 @@ type_new_descriptors(const type_new_ctx *ctx, PyTypeObject *type, PyObject *dict
     if (ctx->add_weak) {
         assert((type->tp_flags & Py_TPFLAGS_MANAGED_WEAKREF) == 0);
         type_add_flags(type, Py_TPFLAGS_MANAGED_WEAKREF);
-        if (_PyType_IS_GC(type)) {
-            type->tp_weaklistoffset = MANAGED_WEAKREF_OFFSET;
-        }
-        else {
-            type->tp_weaklistoffset = MANAGED_WEAKREF_OFFSET_NO_GC;
-        }
+        type->tp_weaklistoffset = MANAGED_WEAKREF_OFFSET;
     }
     if (ctx->add_dict) {
         assert((type->tp_flags & Py_TPFLAGS_MANAGED_DICT) == 0);
@@ -8505,13 +8500,6 @@ overrides_hash(PyTypeObject *type)
     return r;
 }
 
-void
-PyObject_NoGC_Preheader_Del(void *op)
-{
-    size_t presize = _PyType_PreHeaderSize(Py_TYPE(op));
-    PyObject_Free(((char *)op) - presize);
-}
-
 static int
 inherit_slots(PyTypeObject *type, PyTypeObject *base)
 {
@@ -8689,21 +8677,7 @@ inherit_slots(PyTypeObject *type, PyTypeObject *base)
         if ((type->tp_flags & Py_TPFLAGS_HAVE_GC) ==
             (base->tp_flags & Py_TPFLAGS_HAVE_GC)) {
             /* They agree about gc. */
-
-            if ((type->tp_flags & Py_TPFLAGS_PREHEADER) &&
-                type->tp_free == NULL &&
-                base->tp_free == PyObject_Free) {
-                /* Because type has preheader fields, its
-                 * objects will be allocated with those fields
-                 * and it should be take in account when object
-                 * is freed, so we use special tp_free.
-                 */
-                type->tp_free = PyObject_NoGC_Preheader_Del;
-            }
-            else {
-                COPYSLOT(tp_free);
-            }
-
+            COPYSLOT(tp_free);
         }
         else if ((type->tp_flags & Py_TPFLAGS_HAVE_GC) &&
                  type->tp_free == NULL &&
@@ -8919,6 +8893,13 @@ type_ready_preheader(PyTypeObject *type)
                         type->tp_name);
             return -1;
         }
+        if (!(type->tp_flags & Py_TPFLAGS_HAVE_GC)) {
+            PyErr_Format(PyExc_SystemError,
+                        "type %s has the Py_TPFLAGS_MANAGED_DICT flag "
+                        "but not Py_TPFLAGS_HAVE_GC flag",
+                        type->tp_name);
+            return -1;
+        }
         type->tp_dictoffset = -1;
     }
     if (type->tp_flags & Py_TPFLAGS_MANAGED_WEAKREF) {
@@ -8931,12 +8912,14 @@ type_ready_preheader(PyTypeObject *type)
                         type->tp_name);
             return -1;
         }
-        if (_PyType_IS_GC(type)) {
-            type->tp_weaklistoffset = MANAGED_WEAKREF_OFFSET;
-        }
-        else {
-            type->tp_weaklistoffset = MANAGED_WEAKREF_OFFSET_NO_GC;
+        if (!(type->tp_flags & Py_TPFLAGS_HAVE_GC)) {
+            PyErr_Format(PyExc_SystemError,
+                        "type %s has the Py_TPFLAGS_MANAGED_WEAKREF flag "
+                        "but not Py_TPFLAGS_HAVE_GC flag",
+                        type->tp_name);
+            return -1;
         }
+        type->tp_weaklistoffset = MANAGED_WEAKREF_OFFSET;
     }
     return 0;
 }

Python/bytecodes.c

@@ -3638,13 +3638,7 @@ dummy_func(
             _LOAD_ATTR_NONDESCRIPTOR_NO_DICT;
 
         op(_CHECK_ATTR_METHOD_LAZY_DICT, (dictoffset/1, owner -- owner)) {
-            PyObject *borrowed = PyStackRef_AsPyObjectBorrow(owner);
-            Py_ssize_t offset = MANAGED_DICT_OFFSET;
-            if (!PyType_IS_GC(Py_TYPE(borrowed))) {
-                offset = MANAGED_DICT_OFFSET_NO_GC;
-            }
-
-            char *ptr = ((char *)borrowed) + offset + dictoffset;
+            char *ptr = ((char *)PyStackRef_AsPyObjectBorrow(owner)) + MANAGED_DICT_OFFSET + dictoffset;
             PyObject *dict = FT_ATOMIC_LOAD_PTR_ACQUIRE(*(PyObject **)ptr);
             /* This object has a __dict__, just not yet created */
             DEOPT_IF(dict != NULL);