Fix free-threaded GC crash from inherited C-stack refs (issue #515)

CHANGES.rst

@@ -5,11 +5,25 @@
 3.5.4 (unreleased)
 ==================
 
+- Fix a crash (segfault) on free-threaded builds of Python 3.14 and
+  later when the garbage collector runs while a greenlet that was
+  started from a non-empty C-stack-reference state is active. 
+  See `issue 515
+  <https://github.com/python-greenlet/greenlet/issues/515>`_.
+  Thanks to ddorian and Kumar Aditya.
+
+- Fix a potential use-after-free on free-threaded builds of Python 3.14
+  and later when the garbage collector runs while a greenlet is
+  suspended holding a ``_PyCStackRef`` (for example, mid attribute
+  resolution). See
+  `issue 515 <https://github.com/python-greenlet/greenlet/issues/515>`_.
+  Thanks to ddorian and Kumar Aditya.
+
 - Fix a deadlock on free-threaded builds when a greenlet switch happened
   while a ``PyCriticalSection`` was held -- for example inside asyncio's
   ``Task.__step``, which holds one on the running task for the duration of
   the step. See `PR 519 <https://github.com/python-greenlet/greenlet/pull/519/>`. 
-  Thank to ddorian and Kumar Aditya.
+  Thanks to ddorian and Kumar Aditya.
 
 3.5.3 (2026-06-26)
 ==================

src/greenlet/TGreenlet.hpp

@@ -8,6 +8,7 @@
 #include <Python.h>
 
 #include <atomic>
+#include <vector>
 
 #include "greenlet_compiler_compat.hpp"
 #include "greenlet_refs.hpp"
@@ -140,6 +141,12 @@ namespace greenlet
         _PyStackRef* stackpointer;
     #ifdef Py_GIL_DISABLED
         _PyCStackRef* c_stack_refs;
+        // Strong references to the objects held by the _PyCStackRef nodes on
+        // our C stack, snapshotted by operator<< when we suspend so tp_traverse
+        // can keep them alive for the free-threaded GC (capture_c_stack_refs
+        // explains why we snapshot rather than walk the list). Empty while we
+        // run.
+        std::vector<OwnedObject> c_stack_ref_snapshot;
     #endif
 #elif GREENLET_PY312
         int py_recursion_depth;
@@ -171,6 +178,14 @@ namespace greenlet
         // need to be present for the eval loop to work.
         void unexpose_frames();
 
+#if GREENLET_PY314 && defined(Py_GIL_DISABLED)
+        // Take a strong reference to every object held by tstate's _PyCStackRef
+        // list into c_stack_ref_snapshot so tp_traverse can keep them alive
+        // while we're suspended. Must run while our C stack is still live
+        // (operator<<). The snapshot is dropped by clear()ing the vector.
+        void capture_c_stack_refs(const PyThreadState* tstate) noexcept;
+#endif
+
     public:
 
         PythonState();

src/greenlet/TPythonState.cpp

@@ -92,6 +92,30 @@ PythonState::PythonState()
 #endif
 }
 
+#if GREENLET_PY314 && defined(Py_GIL_DISABLED)
+void PythonState::capture_c_stack_refs(const PyThreadState* tstate) noexcept
+{
+    // Runs from operator<< while our C stack is still live and coherent, so we
+    // can walk tstate's _PyCStackRef list and take a strong reference to every
+    // object it holds. tp_traverse visits these once we're suspended, because
+    // by then the nodes themselves have been relocated into the heap stack copy
+    // and the saved list head no longer points at them. Strong references (not
+    // _Py_VISIT_STACKREF, whose _PyGC_VisitStackRef isn't exported before 3.15);
+    // a std::vector rather than a Python list/tuple because operator<< must not
+    // allocate a GC-tracked object mid-switch. Rebuilt from scratch each time;
+    // the list is empty at a typical switch, so this is usually just an empty
+    // loop.
+    this->c_stack_ref_snapshot.clear();
+    for (const _PyCStackRef* node = ((_PyThreadStateImpl*)tstate)->c_stack_refs;
+         node != nullptr; node = node->next) {
+        if (!PyStackRef_IsNullOrInt(node->ref)) {
+            this->c_stack_ref_snapshot.push_back(
+                OwnedObject::owning(PyStackRef_AsPyObjectBorrow(node->ref)));
+        }
+    }
+}
+#endif
+
 
 inline void PythonState::may_switch_away() noexcept
 {
@@ -145,6 +169,9 @@ void PythonState::operator<<(const PyThreadState *const tstate) noexcept
     this->current_executor = tstate->current_executor;
     #ifdef Py_GIL_DISABLED
     this->c_stack_refs = ((_PyThreadStateImpl*)tstate)->c_stack_refs;
+    // Capture the deferred references now, while our C stack is still live, so
+    // tp_traverse can keep them from being collected while we're suspended.
+    this->capture_c_stack_refs(tstate);
     #endif
   #elif GREENLET_PY312
     this->py_recursion_depth = tstate->py_recursion_limit - tstate->py_recursion_remaining;
@@ -265,6 +292,10 @@ void PythonState::operator>>(PyThreadState *const tstate) noexcept
     tstate->current_executor = this->current_executor;
     #ifdef Py_GIL_DISABLED
     ((_PyThreadStateImpl*)tstate)->c_stack_refs = this->c_stack_refs;
+    // We're the running greenlet again: our C-stack refs live in the thread
+    // state now and gc_visit_thread_stacks() covers them, so drop the strong
+    // references tp_traverse held on our behalf while we were suspended.
+    this->c_stack_ref_snapshot.clear();
     #endif
     this->unexpose_frames();
   #elif GREENLET_PY312
@@ -355,7 +386,14 @@ void PythonState::set_initial_state(const PyThreadState* const tstate) noexcept
     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;
+    // Start with an empty C-stack-ref list, the way a brand-new thread does;
+    // do NOT copy the parent thread state's head. Those _PyCStackRef nodes sit
+    // on the parent greenlet's C stack, so once we start running on our own
+    // stack and overwrite that region, following them reads garbage. The
+    // free-threaded collector walks c_stack_refs for every thread in
+    // gc_visit_thread_stacks(), so leaving the stale head here crashed it.
+    // See https://github.com/python-greenlet/greenlet/issues/515.
+    this->c_stack_refs = nullptr;
     #endif
     // this->stackpointer is left null because this->_top_frame is
     // null so there is no value to copy.
@@ -405,6 +443,18 @@ int PythonState::tp_traverse(visitproc visit, void* arg, bool visit_top_frame) n
             }
         }
     }
+#endif
+#if GREENLET_PY314 && defined(Py_GIL_DISABLED)
+    // Visit the objects this greenlet's C-stack refs were holding when it
+    // suspended (captured by capture_c_stack_refs). The free-threaded collector
+    // only walks the running thread's _PyCStackRef list in
+    // gc_visit_thread_stacks(), so without this a collection could free an
+    // object reachable only through a suspended greenlet's C-stack ref and we'd
+    // use it after free once the greenlet resumed. The snapshot is empty while
+    // we're the running greenlet, so this is a no-op there.
+    for (const OwnedObject& ref : this->c_stack_ref_snapshot) {
+        Py_VISIT(ref.borrow());
+    }
 #endif
     // Note that we DO NOT visit ``delete_later``. Even if it's
     // non-null and we technically own a reference to it, its
@@ -418,6 +468,9 @@ int PythonState::tp_traverse(visitproc visit, void* arg, bool visit_top_frame) n
 void PythonState::tp_clear(bool own_top_frame) noexcept
 {
     PythonStateContext::tp_clear();
+#if GREENLET_PY314 && defined(Py_GIL_DISABLED)
+    this->c_stack_ref_snapshot.clear();
+#endif
     // If we get here owning a frame,
     // we got dealloc'd without being finished. We may or may not be
     // in the same thread.

src/greenlet/tests/fail_c_stack_refs_suspended_gc.py

@@ -0,0 +1,77 @@
+"""Regression test for GC of a suspended greenlet's C-stack refs (issue #515).
+
+When a greenlet suspends while the interpreter is holding a ``_PyCStackRef``
+(for example, mid attribute resolution), that deferred reference sits on the
+greenlet's C stack. The free-threaded collector only walks the running
+thread's list in ``gc_visit_thread_stacks``, so unless greenlet visits the
+suspended one from ``tp_traverse`` an object reachable only through it is freed
+early and used after free once the greenlet resumes.
+
+Reproducing that needs the pinned object to be deferred-refcounted (so its
+refcount doesn't keep it alive) and reachable only through the C-stack ref. A
+class is deferred-refcounted, and a metaclass ``__get__`` makes a class act as a
+descriptor -- so ``obj.attr``, where ``attr`` is such a class, pins the class in
+a ``_PyCStackRef`` across the (Python) ``__get__``. We switch away from inside
+``__get__``, drop every other reference to the class, and collect: on a
+regressed free-threaded build the class is gone by the time we resume; the fix
+keeps it alive because ``tp_traverse`` visited the snapshot taken at suspend.
+
+Prints "C STACK REFS GC OK" on a fixed build (and on with-GIL builds, where
+ordinary refcounting keeps the class alive); a regressed free-threaded build
+reports the class was collected and exits non-zero.
+"""
+import gc
+import sys
+import weakref
+
+import greenlet
+
+parent = greenlet.getcurrent()
+observed = {}
+
+
+class Meta(type):
+    def __get__(cls, obj, objtype=None):
+        # type(cls) is Meta, which defines __get__, so ``cls`` is a descriptor;
+        # the getattr machinery pins it in a _PyCStackRef across this call. A
+        # class is deferred-refcounted on a free-threaded build, so only that
+        # (deferred) C-stack ref will be keeping it alive in a moment.
+        #
+        # Drop the locals the descriptor protocol gave us (the class is ``cls``):
+        # greenlet visits a suspended greenlet's frames, so a leftover frame ref
+        # would keep the class alive on its own and mask the bug.
+        del cls, obj, objtype
+        child.switch()
+        return 42
+
+
+pinned = Meta('pinned', (), {})                    # a class => deferred-refcounted
+holder = type('holder', (), {'attr': pinned})      # holder.attr invokes Meta.__get__
+box = [pinned]
+del pinned
+
+
+def child_work():
+    # ``parent`` is suspended inside Meta.__get__, holding a C-stack ref to the
+    # class. Drop every *other* reference to it, then collect: now only greenlet
+    # visiting the suspended C-stack ref can keep the class alive.
+    ref = weakref.ref(box[0])
+    box[0] = None
+    del holder.attr
+    for _ in range(5):
+        gc.collect()
+    observed['alive'] = ref() is not None
+    parent.switch()
+
+
+child = greenlet.greenlet(child_work)
+result = holder().attr                             # Meta.__get__ -> switch -> gc -> back
+assert result == 42, result
+
+alive = observed.get('alive')
+print(f"py={sys.version.split()[0]} gil={getattr(sys, '_is_gil_enabled', lambda: True)()} "
+      f"greenlet={greenlet.__version__} alive={alive}", flush=True)
+if not alive:
+    raise SystemExit("REGRESSED: a class reachable only through a suspended "
+                     "greenlet's C-stack ref was collected early")
+print("C STACK REFS GC OK", flush=True)

src/greenlet/tests/fail_issue_515_freethread_gc.py

@@ -0,0 +1,112 @@
+"""Reproducer for the free-threaded GC crash in issue #515.
+
+https://github.com/python-greenlet/greenlet/issues/515
+
+Before the fix, ``set_initial_state`` copied the parent thread state's
+``_PyCStackRef`` list head into each new greenlet. Those nodes sit on the
+parent greenlet's C stack, so once the child starts running on its own stack
+and overwrites that region their ``next`` pointers dangle. The free-threaded
+collector walks ``c_stack_refs`` for every thread, so a GC on any thread ends
+up chasing those dangling nodes (inside ``gc_collect_main`` ->
+``gc_visit_thread_stacks``) while such a child is active, and segfaults.
+
+The fiddly part is getting a *non-empty* list to inherit at all. The
+interpreter only holds a ``_PyCStackRef`` transiently, while it is resolving an
+attribute -- ``_Py_LoadAttr_StackRefSteal`` pins ``self`` across ``obj.attr()``
+-- so the greenlet has to be primed from inside that window. Priming it from a
+property getter does exactly that; it is also the same deep-attribute context
+Playwright's sync bridge happens to start its fiber from. A churn thread then
+keeps the collector busy while the dispatcher runs on its stale list.
+
+A fixed build prints "ISSUE 515 OK"; a regressed one segfaults in about a
+second. With-GIL builds are unaffected -- ordinary refcounting keeps the nodes
+alive -- so there it simply prints the OK line.
+"""
+import gc
+import os
+import sys
+import threading
+
+import greenlet
+
+# The crash lands within a second, so a few seconds leaves plenty of margin on
+# a slow machine without dragging out a fixed build.
+DURATION = float(os.environ.get("REPRO_SECONDS", "4"))
+NWORKERS = int(os.environ.get("REPRO_THREADS", "6"))
+
+
+def _busy():
+    # Keep the dispatcher doing work (resolving str methods) so a collection on
+    # the churn thread catches it while it is the active greenlet.
+    return "abc".upper().lower().strip().title()
+
+
+class Bridge:
+    def __init__(self, stop):
+        self.stop = stop
+        self.main = greenlet.getcurrent()
+        self.disp = greenlet.greenlet(self._loop)
+
+    def _loop(self):
+        while not self.stop[0]:
+            for _ in range(50):
+                _busy()
+            self.main.switch()
+
+    @property
+    def _prime(self):
+        # Reaching this getter means we are mid attribute-resolution, so a
+        # _PyCStackRef for ``self`` is live right now. Priming the dispatcher
+        # from here is what makes it inherit a non-empty c_stack_refs head.
+        self.disp.switch()
+        return None
+
+    def start(self):
+        # Reading the property (rather than calling a method) is deliberate: the
+        # getter runs *during* attribute resolution, so it primes the dispatcher
+        # while the _PyCStackRef for ``self`` is still held. We return the value
+        # only so this isn't a bare, pointless-looking expression statement.
+        return self._prime
+
+    def resume(self):
+        self.disp.switch()
+
+
+def worker(stop):
+    bridge = Bridge(stop)
+    bridge.start()
+    # start() has returned, so the stack the inherited nodes point at is being
+    # reused -- the dispatcher's list is stale now. Keep it active to trip the GC.
+    while not stop[0]:
+        bridge.resume()
+
+
+def churn(stop):
+    while not stop[0]:
+        # A cycle forces a real (stop-the-world) collection rather than just a
+        # refcount drop.
+        nodes = [{"i": i, "next": None} for i in range(500)]
+        for a, b in zip(nodes, nodes[1:]):
+            a["next"] = b
+        nodes[-1]["next"] = nodes[0]
+        del nodes
+        gc.collect()
+
+
+def main():
+    stop = [False]
+    threads = [threading.Thread(target=churn, args=(stop,), daemon=True)]
+    threads += [threading.Thread(target=worker, args=(stop,), daemon=True)
+                for _ in range(NWORKERS)]
+    for t in threads:
+        t.start()
+    threading.Event().wait(DURATION)
+    stop[0] = True
+    for t in threads:
+        t.join(timeout=5)
+    print("ISSUE 515 OK")
+
+
+if __name__ == "__main__":
+    main()
+    sys.exit(0)

src/greenlet/tests/test_gc.py

@@ -6,6 +6,7 @@
 
 
 from . import TestCase
+from . import RUNNING_ON_FREETHREAD_BUILD
 from .leakcheck import fails_leakcheck_on_py314_or_less
 # These only work with greenlet gc support
 # which is no longer optional.
@@ -84,6 +85,32 @@ def greenlet_body():
         greenlet.getcurrent()
         gc.collect()
 
+    def test_issue515_freethread_c_stack_refs(self):
+        # Guards issue #515: a new greenlet inherited the parent's C-stack refs
+        # and the free-threaded collector segfaulted following the dangling
+        # nodes. This has to run out of process because the regression is a hard
+        # crash, not something we can catch.
+        # https://github.com/python-greenlet/greenlet/issues/515
+        if not RUNNING_ON_FREETHREAD_BUILD:
+            self.skipTest("Only free-threaded builds are affected")
+        output = self.run_script('fail_issue_515_freethread_gc.py')
+        self.assertIn('ISSUE 515 OK', output)
+
+    def test_c_stack_refs_suspended_gc(self):
+        # Review follow-up to #515: a greenlet that suspends while holding a
+        # _PyCStackRef has those deferred references visited by tp_traverse (the
+        # snapshot in TPythonState.cpp), so the free-threaded collector can't
+        # free an object reachable only through the suspended greenlet's C stack.
+        # The script pins a (deferred-refcounted) class via a metaclass __get__,
+        # switches away from inside it, drops every other reference, and
+        # collects; without the fix the class is gone on resume. Out of process
+        # because the regression is a use-after-free.
+        # https://github.com/python-greenlet/greenlet/issues/515
+        if not RUNNING_ON_FREETHREAD_BUILD:
+            self.skipTest("Only free-threaded builds are affected")
+        output = self.run_script('fail_c_stack_refs_suspended_gc.py')
+        self.assertIn('C STACK REFS GC OK', output)
+
     def test_crashing_deferred_object(self):
         if sys.version_info < (3, 15):
             self.skipTest("Test is 3.15+ only")