Fix some crashes with CPython assertions enabled, pointing to things we were doing wrong it free-threading.

Specifically, we need to preserve the c_stack_refs attribute, and be careful about what we say can be GC'd.

Author: jamadden

.github/workflows/tests.yml

@@ -51,8 +51,8 @@ jobs:
     - name: Install greenlet (non-Mac)
       if: ${{ ! startsWith(runner.os, 'Mac') }}
       run: |
-        # Stupid setuptools doesn't want you running 'python setup.py' anymore,
-        # but stupid pip hides all the intersting compiler output by default, and the
+        # setuptools doesn't want you running 'python setup.py' anymore,
+        # but pip hides all the intersting compiler output by default, and the
         # only way to get anything useful out is to ask *everything* to be verbose,
         # which is much more junk than we need to wade through, making it hard to
         # see what we want. What's the point of having warnings at all if we can't
@@ -104,6 +104,20 @@ jobs:
         python -c 'import greenlet._greenlet as G; assert G.GREENLET_USE_STANDARD_THREADING'
         python -m unittest discover -v greenlet.tests
     - name: Doctest
+      env:
+        # XXX: On 3.14t, when the thread-local bytecode cache is
+        # enabled, sphinx crashes on module cleanup: there is a
+        # reference to pdb.set_trace in ``glob``, and when the
+        # shutdown sequence tries to clear that value, it segfaults
+        # dec-reffing it after taking it out of the module dict. The
+        # object appears to be corrupt, but it is utterly unclear how
+        # we could have done this. It is 100% reliable on bath Mac and
+        # Linux. It can be traced down to a very simple doctest
+        # snippet in greenlet_gc.rst, but running that same snippet
+        # standalone in a unit test doesn't produce the error.
+        #
+        # So this is a temporary workaround.
+        PYTHON_TLBC: "0"
       run: |
         sphinx-build -b doctest -d docs/_build/doctrees2 docs docs/_build/doctest2
     - name: Lint

src/greenlet/PyGreenlet.cpp

@@ -741,7 +741,30 @@ PyTypeObject PyGreenlet_Type = {
     .tp_alloc=PyType_GenericAlloc,                  /* tp_alloc */
     .tp_new=(newfunc)green_new,                          /* tp_new */
     .tp_free=PyObject_GC_Del,                   /* tp_free */
+#ifndef Py_GIL_DISABLED
+/*
+  We may have been handling this wrong all along.
+
+  It shows as a problem with the GIL disabled. In builds of 3.14 with
+  assertions enabled, we break the garbage collector if we *ever*
+  return false from this function. The docs say this is to distinguish
+  some objects that are collectable vs some that are not, specifically
+  giving the example of PyTypeObject as the only place this is done,
+  where it distinguishes between static types like this one (allocated
+  by the C runtime at load time) and dynamic heap types (created at
+  runtime as objects). With the GIL disabled, all allocations that are
+  potentially collectable go in the mimalloc heap, and the collector
+  asserts that tp_is_gc() is true for them as it walks through the
+  heap object by object. Since we set the Py_TPFLAGS_HAS_GC bit, we
+  are always allocated in that mimalloc heap, so we must always be
+  collectable.
+
+  XXX: TODO: Could this be responsible for some apparent leaks, even
+  on GIL builds, at least in 3.14? See if we can catch an assertion
+  failure in the GC on regular 3.14 as well.
+ */
     .tp_is_gc=(inquiry)green_is_gc,         /* tp_is_gc */
+#endif
 };
 
 #endif

src/greenlet/TGreenlet.hpp

@@ -34,6 +34,9 @@ using greenlet::refs::BorrowedGreenlet;
 #else
 #  include "internal/pycore_interpframe.h"
 #endif
+#ifdef Py_GIL_DISABLED
+#   include "internal/pycore_tstate.h"
+#endif
 #endif
 
 // XXX: TODO: Work to remove all virtual functions
@@ -122,6 +125,9 @@ namespace greenlet
         //      Assertion `tstate->current_executor == NULL' failed.
         // see https://github.com/python-greenlet/greenlet/issues/460
         PyObject* current_executor;
+    #ifdef Py_GIL_DISABLED
+        _PyCStackRef* c_stack_refs;
+    #endif
 #elif GREENLET_PY312
         int py_recursion_depth;
         int c_recursion_depth;

src/greenlet/TPythonState.cpp

@@ -15,6 +15,9 @@ PythonState::PythonState()
 #if GREENLET_PY314
     ,py_recursion_depth(0)
     ,current_executor(nullptr)
+    #ifdef Py_GIL_DISABLED
+    ,c_stack_refs(nullptr)
+    #endif
 #elif GREENLET_PY312
     ,py_recursion_depth(0)
     ,c_recursion_depth(0)
@@ -138,6 +141,9 @@ void PythonState::operator<<(const PyThreadState *const tstate) noexcept
   #if GREENLET_PY314
     this->py_recursion_depth = tstate->py_recursion_limit - tstate->py_recursion_remaining;
     this->current_executor = tstate->current_executor;
+    #ifdef Py_GIL_DISABLED
+    this->c_stack_refs = ((_PyThreadStateImpl*)tstate)->c_stack_refs;
+    #endif
   #elif GREENLET_PY312
     this->py_recursion_depth = tstate->py_recursion_limit - tstate->py_recursion_remaining;
     this->c_recursion_depth = Py_C_RECURSION_LIMIT - tstate->c_recursion_remaining;
@@ -216,6 +222,9 @@ void PythonState::operator>>(PyThreadState *const tstate) noexcept
   #if GREENLET_PY314
     tstate->py_recursion_remaining = tstate->py_recursion_limit - this->py_recursion_depth;
     tstate->current_executor = this->current_executor;
+    #ifdef Py_GIL_DISABLED
+    ((_PyThreadStateImpl*)tstate)->c_stack_refs = this->c_stack_refs;
+    #endif
     this->unexpose_frames();
   #elif GREENLET_PY312
     tstate->py_recursion_remaining = tstate->py_recursion_limit - this->py_recursion_depth;

src/greenlet/greenlet_allocator.hpp

@@ -70,6 +70,7 @@ namespace greenlet
         };
 
     };
+
 }
 
 #endif

src/greenlet/tests/test_gc.py

@@ -11,7 +11,7 @@
 # which is no longer optional.
 assert greenlet.GREENLET_USE_GC
 
-class GCTests(TestCase):
+class TestGC(TestCase):
     def test_dead_circular_ref(self):
         o = weakref.ref(greenlet.greenlet(greenlet.getcurrent).switch())
         gc.collect()
@@ -84,3 +84,26 @@ def greenlet_body():
         del g
         greenlet.getcurrent()
         gc.collect()
+
+    # def test_crashing_cycle(self):
+    #     # CPython 3.14 free threaded crashes on this
+    #     # (from docs/greenlet_gc.rst)
+    #     import doctest
+
+    #     def with_doctest():
+    #         """
+    #         >>> import gc
+    #         >>> from greenlet import getcurrent, greenlet, GreenletExit
+    #         >>> def collect_it():
+    #         ...      print("Collecting garbage")
+    #         ...      gc.collect()
+    #         >>> def outer():
+    #         ...     gc.collect()
+    #         >>> outer_glet = greenlet(outer)
+    #         >>> outer_glet.switch()
+    #         Collecting garbage
+    #         >>> print(list(globals()))
+    #         >>> print(list(locals()))
+    #         """
+
+    #     doctest.run_docstring_examples(with_doctest, dict())