Adding tp_reachable (#65)

Includes various fixes to get more passing CI too

Co-authored-by: Fridtjof Stoldt <xFrednet@gmail.com>

Author: mjp41

Doc/includes/typestruct.h

@@ -92,4 +92,7 @@ typedef struct _typeobject {
      * Otherwise, limited to MAX_VERSIONS_PER_CLASS (defined elsewhere).
      */
     uint16_t tp_versions_used;
+
+    /* call function for all referenced objects (includes non-cyclic refs) */
+    traverseproc tp_reachable;
 } PyTypeObject;

Include/cpython/object.h

@@ -239,6 +239,9 @@ struct _typeobject {
      * Otherwise, limited to MAX_VERSIONS_PER_CLASS (defined elsewhere).
      */
     uint16_t tp_versions_used;
+
+    /* call function for all referenced objects (includes non-cyclic refs) */
+    traverseproc tp_reachable;
 };
 
 #define _Py_ATTR_CACHE_UNUSED (30000)  // (see tp_versions_used)

Include/internal/pycore_immutability.h

@@ -8,11 +8,14 @@ extern "C" {
 #  error "Py_BUILD_CORE must be defined to include this header"
 #endif
 
+typedef struct _Py_hashtable_t _Py_hashtable_t;
+
 struct _Py_immutability_state {
     PyObject *module_locks;
     PyObject *blocking_on;
     PyObject *freezable_types;
     PyObject *destroy_cb;
+    _Py_hashtable_t *warned_types;
 #ifdef Py_DEBUG
     PyObject *traceback_func;  // For debugging purposes, can be NULL
 #endif

Include/typeslots.h

@@ -94,3 +94,7 @@
 /* New in 3.14 */
 #define Py_tp_token 83
 #endif
+#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030F0000
+/* New in 3.15 */
+#define Py_tp_reachable 84
+#endif

Lib/test/test_freeze/test_common.py

@@ -8,6 +8,9 @@ def __init__(self, *args, obj=None, **kwargs):
         self.obj = obj
 
     def setUp(self):
+        # Explicitly freeze type, then the object
+        # Types are not implicitly frozen by freeze()
+        # freeze(type(self.obj))
         freeze(self.obj)
 
     def test_immutable(self):
@@ -18,7 +21,8 @@ def test_add_attribute(self):
             self.obj.new_attribute = 'value'
 
     def test_type_immutable(self):
-        self.assertTrue(isfrozen(type(self.obj)))
+        self.assertTrue(isfrozen(self.obj))
+        self.assertTrue(isfrozen(type(self.obj)), "Type should be frozen when instance is frozen: {}".format(type(self.obj)))
 
 
 class BaseNotFreezableTest(unittest.TestCase):

Lib/test/test_freeze/test_reachable_warnings.py

@@ -0,0 +1,89 @@
+"""Tests for freeze warnings when tp_reachable is missing.
+
+Uses the _test_reachable C extension which provides two static types:
+
+  HasTraverseNoReachable  – has tp_traverse, tp_reachable deliberately NULL
+  NoTraverseNoReachable   – neither tp_traverse nor tp_reachable
+"""
+import subprocess
+import sys
+import textwrap
+import unittest
+
+
+class TestReachableWarnings(unittest.TestCase):
+    """Test that freeze logs warnings when tp_reachable is missing."""
+
+    def _run_code(self, code):
+        """Run code in a subprocess and return (stdout, stderr)."""
+        result = subprocess.run(
+            [sys.executable, "-c", textwrap.dedent(code)],
+            capture_output=True,
+            text=True,
+        )
+        self.assertEqual(result.returncode, 0, result.stderr)
+        return result.stdout, result.stderr
+
+    def test_warn_tp_traverse_no_tp_reachable(self):
+        """Warn when a C type has tp_traverse but no tp_reachable."""
+        stdout, stderr = self._run_code("""\
+            import _immutable, _test_reachable
+            obj = _test_reachable.HasTraverseNoReachable(42)
+            _immutable.freeze(obj)
+        """)
+        self.assertIn(
+            "freeze: type '_test_reachable.HasTraverseNoReachable' "
+            "has tp_traverse but no tp_reachable",
+            stderr,
+        )
+
+    def test_warn_no_traverse_no_reachable(self):
+        """Warn when a C type has neither tp_traverse nor tp_reachable."""
+        stdout, stderr = self._run_code("""\
+            import _immutable, _test_reachable
+            obj = _test_reachable.NoTraverseNoReachable()
+            _immutable.freeze(obj)
+        """)
+        self.assertIn(
+            "freeze: type '_test_reachable.NoTraverseNoReachable' "
+            "has no tp_traverse and no tp_reachable",
+            stderr,
+        )
+
+    def test_warn_only_once_per_type(self):
+        """A type should only produce the warning on the first freeze."""
+        stdout, stderr = self._run_code("""\
+            import _immutable, _test_reachable
+            _immutable.freeze(_test_reachable.HasTraverseNoReachable(1))
+            _immutable.freeze(_test_reachable.HasTraverseNoReachable(2))
+            _immutable.freeze(_test_reachable.HasTraverseNoReachable(3))
+        """)
+        msg = (
+            "freeze: type '_test_reachable.HasTraverseNoReachable' "
+            "has tp_traverse but no tp_reachable"
+        )
+        count = stderr.count(msg)
+        self.assertEqual(count, 1, f"Expected 1 warning, got {count}:\n{stderr}")
+
+    def test_warn_different_types_separately(self):
+        """Different types should each produce their own warning."""
+        stdout, stderr = self._run_code("""\
+            import _immutable, _test_reachable
+            _immutable.freeze(_test_reachable.HasTraverseNoReachable(1))
+            _immutable.freeze(_test_reachable.NoTraverseNoReachable())
+        """)
+        self.assertIn("HasTraverseNoReachable", stderr)
+        self.assertIn("NoTraverseNoReachable", stderr)
+
+    def test_no_warning_with_tp_reachable(self):
+        """No warning for a type that has tp_reachable set."""
+        stdout, stderr = self._run_code("""\
+            import _immutable, _test_reachable
+            obj = _test_reachable.HasReachable(42)
+            _immutable.freeze(obj)
+        """)
+        self.assertNotIn("HasReachable", stderr)
+
+
+if __name__ == "__main__":
+    unittest.main()

Lib/test/test_sys.py

@@ -1782,7 +1782,7 @@ def delx(self): del self.__x
         check((1,2,3), vsize('') + self.P + 3*self.P)
         # type
         # static type: PyTypeObject
-        fmt = 'P2nPI13Pl4Pn9Pn12PIPc'
+        fmt = 'P2nPI13Pl4Pn9Pn12PIPcP'
         s = vsize(fmt)
         check(int, s)
         typeid = 'n' if support.Py_GIL_DISABLED else ''

Modules/Setup.stdlib.in

@@ -179,6 +179,7 @@
 @MODULE__TESTLIMITEDCAPI_TRUE@_testlimitedcapi _testlimitedcapi.c _testlimitedcapi/abstract.c _testlimitedcapi/bytearray.c _testlimitedcapi/bytes.c _testlimitedcapi/codec.c _testlimitedcapi/complex.c _testlimitedcapi/dict.c _testlimitedcapi/eval.c _testlimitedcapi/float.c _testlimitedcapi/heaptype_relative.c _testlimitedcapi/import.c _testlimitedcapi/list.c _testlimitedcapi/long.c _testlimitedcapi/object.c _testlimitedcapi/pyos.c _testlimitedcapi/set.c _testlimitedcapi/sys.c _testlimitedcapi/tuple.c _testlimitedcapi/unicode.c _testlimitedcapi/vectorcall_limited.c _testlimitedcapi/version.c _testlimitedcapi/file.c
 @MODULE__TESTCLINIC_TRUE@_testclinic _testclinic.c
 @MODULE__TESTCLINIC_LIMITED_TRUE@_testclinic_limited _testclinic_limited.c
+@MODULE__TEST_REACHABLE_TRUE@_test_reachable _test_reachable.c
 
 # Some testing modules MUST be built as shared libraries.
 *shared*

Modules/_test_reachable.c

@@ -0,0 +1,220 @@
+/*
+ * _test_reachable - Test module for tp_reachable freeze warnings.
+ *
+ * Provides types that deliberately lack tp_reachable so we can test
+ * the freeze-time warnings in traverse_freeze().
+ *
+ *   HasTraverseNoReachable  - has tp_traverse, no tp_reachable
+ *   NoTraverseNoReachable   - no  tp_traverse, no tp_reachable
+ *   HasReachable            - has both tp_traverse and tp_reachable (no warning)
+ */
+
+#ifndef Py_BUILD_CORE_BUILTIN
+#  define Py_BUILD_CORE_MODULE 1
+#endif
+
+#include "Python.h"
+
+/* ---- HasTraverseNoReachable ------------------------------------------- */
+
+typedef struct {
+    PyObject_HEAD
+    PyObject *value;
+} HasTraverseNoReachableObject;
+
+static int
+htnr_traverse(PyObject *self, visitproc visit, void *arg)
+{
+    HasTraverseNoReachableObject *obj = (HasTraverseNoReachableObject *)self;
+    Py_VISIT(obj->value);
+    return 0;
+}
+
+static int
+htnr_clear(PyObject *self)
+{
+    HasTraverseNoReachableObject *obj = (HasTraverseNoReachableObject *)self;
+    Py_CLEAR(obj->value);
+    return 0;
+}
+
+static void
+htnr_dealloc(PyObject *self)
+{
+    PyObject_GC_UnTrack(self);
+    htnr_clear(self);
+    Py_TYPE(self)->tp_free(self);
+}
+
+static int
+htnr_init(PyObject *self, PyObject *args, PyObject *kwds)
+{
+    HasTraverseNoReachableObject *obj = (HasTraverseNoReachableObject *)self;
+    PyObject *value = Py_None;
+    if (!PyArg_ParseTuple(args, "|O", &value))
+        return -1;
+    Py_XSETREF(obj->value, Py_NewRef(value));
+    return 0;
+}
+
+static PyTypeObject HasTraverseNoReachable_Type = {
+    PyVarObject_HEAD_INIT(NULL, 0)
+    .tp_name = "_test_reachable.HasTraverseNoReachable",
+    .tp_basicsize = sizeof(HasTraverseNoReachableObject),
+    .tp_dealloc = htnr_dealloc,
+    .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC,
+    .tp_doc = "Type with tp_traverse but no tp_reachable.",
+    .tp_traverse = htnr_traverse,
+    .tp_clear = htnr_clear,
+    .tp_init = htnr_init,
+    .tp_alloc = PyType_GenericAlloc,
+    .tp_new = PyType_GenericNew,
+    .tp_free = PyObject_GC_Del,
+    /* tp_reachable deliberately left NULL */
+};
+
+/* ---- NoTraverseNoReachable ------------------------------------------- */
+
+typedef struct {
+    PyObject_HEAD
+    int dummy;
+} NoTraverseNoReachableObject;
+
+static int
+ntnr_init(PyObject *self, PyObject *args, PyObject *kwds)
+{
+    return 0;
+}
+
+static PyTypeObject NoTraverseNoReachable_Type = {
+    PyVarObject_HEAD_INIT(NULL, 0)
+    .tp_name = "_test_reachable.NoTraverseNoReachable",
+    .tp_basicsize = sizeof(NoTraverseNoReachableObject),
+    .tp_flags = Py_TPFLAGS_DEFAULT,
+    .tp_doc = "Type with no tp_traverse and no tp_reachable.",
+    .tp_init = ntnr_init,
+    .tp_alloc = PyType_GenericAlloc,
+    .tp_new = PyType_GenericNew,
+    /* tp_traverse deliberately left NULL */
+    /* tp_reachable deliberately left NULL */
+};
+
+/* ---- HasReachable ---------------------------------------------------- */
+
+typedef struct {
+    PyObject_HEAD
+    PyObject *value;
+} HasReachableObject;
+
+static int
+hr_traverse(PyObject *self, visitproc visit, void *arg)
+{
+    HasReachableObject *obj = (HasReachableObject *)self;
+    Py_VISIT(obj->value);
+    return 0;
+}
+
+static int
+hr_reachable(PyObject *self, visitproc visit, void *arg)
+{
+    Py_VISIT(Py_TYPE(self));
+    return hr_traverse(self, visit, arg);
+}
+
+static int
+hr_clear(PyObject *self)
+{
+    HasReachableObject *obj = (HasReachableObject *)self;
+    Py_CLEAR(obj->value);
+    return 0;
+}
+
+static void
+hr_dealloc(PyObject *self)
+{
+    PyObject_GC_UnTrack(self);
+    hr_clear(self);
+    Py_TYPE(self)->tp_free(self);
+}
+
+static int
+hr_init(PyObject *self, PyObject *args, PyObject *kwds)
+{
+    HasReachableObject *obj = (HasReachableObject *)self;
+    PyObject *value = Py_None;
+    if (!PyArg_ParseTuple(args, "|O", &value))
+        return -1;
+    Py_XSETREF(obj->value, Py_NewRef(value));
+    return 0;
+}
+
+static PyTypeObject HasReachable_Type = {
+    PyVarObject_HEAD_INIT(NULL, 0)
+    .tp_name = "_test_reachable.HasReachable",
+    .tp_basicsize = sizeof(HasReachableObject),
+    .tp_dealloc = hr_dealloc,
+    .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC,
+    .tp_doc = "Type with both tp_traverse and tp_reachable.",
+    .tp_traverse = hr_traverse,
+    .tp_clear = hr_clear,
+    .tp_init = hr_init,
+    .tp_alloc = PyType_GenericAlloc,
+    .tp_new = PyType_GenericNew,
+    .tp_free = PyObject_GC_Del,
+    .tp_reachable = hr_reachable,
+};
+
+/* ---- Module ---------------------------------------------------------- */
+
+static int
+_test_reachable_exec(PyObject *module)
+{
+    /* Ready both types and register them as freezable */
+    if (PyType_Ready(&HasTraverseNoReachable_Type) < 0)
+        return -1;
+    /* Reset tp_reachable to NULL in case Ready inherited one */
+    HasTraverseNoReachable_Type.tp_reachable = NULL;
+    if (PyModule_AddType(module, &HasTraverseNoReachable_Type) != 0)
+        return -1;
+    if (_PyImmutability_RegisterFreezable(&HasTraverseNoReachable_Type) < 0)
+        return -1;
+
+    if (PyType_Ready(&NoTraverseNoReachable_Type) < 0)
+        return -1;
+    /* Reset tp_reachable to NULL in case Ready inherited one */
+    NoTraverseNoReachable_Type.tp_reachable = NULL;
+    if (PyModule_AddType(module, &NoTraverseNoReachable_Type) != 0)
+        return -1;
+    if (_PyImmutability_RegisterFreezable(&NoTraverseNoReachable_Type) < 0)
+        return -1;
+
+    if (PyType_Ready(&HasReachable_Type) < 0)
+        return -1;
+    if (PyModule_AddType(module, &HasReachable_Type) != 0)
+        return -1;
+    if (_PyImmutability_RegisterFreezable(&HasReachable_Type) < 0)
+        return -1;
+
+    return 0;
+}
+
+static PyModuleDef_Slot _test_reachable_slots[] = {
+    {Py_mod_exec, _test_reachable_exec},
+    {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
+    {Py_mod_gil, Py_MOD_GIL_NOT_USED},
+    {0, NULL},
+};
+
+static struct PyModuleDef _test_reachable_module = {
+    PyModuleDef_HEAD_INIT,
+    .m_name = "_test_reachable",
+    .m_doc = "Test module for tp_reachable freeze warnings.",
+    .m_size = 0,
+    .m_slots = _test_reachable_slots,
+};
+
+PyMODINIT_FUNC
+PyInit__test_reachable(void)
+{
+    return PyModuleDef_Init(&_test_reachable_module);
+}