From 14e24b27f5f72fec9aebb91c2e876d01b8f9c917 Mon Sep 17 00:00:00 2001 From: Dorian Hoxha Date: Fri, 3 Jul 2026 16:32:43 +0200 Subject: [PATCH 1/5] Fix free-threaded GC crash from inherited C-stack refs (issue #515) set_initial_state copied the parent thread state's _PyCStackRef list head into every newly-started greenlet. Those nodes live on the parent greenlet's C stack, so once the child ran on its own stack and overwrote that region the next pointers dangled. The free-threaded collector walks c_stack_refs for every thread in gc_visit_thread_stacks(), so a collection on any thread would follow the dangling nodes and segfault while a child greenlet was active (fault inside gc_collect_main). This reproduced on 3.14t and 3.15t alike. Start new greenlets with an empty C-stack-ref list, the way a fresh thread does. Adds a pure-greenlet regression test that crashes a regressed build and runs clean once fixed. --- CHANGES.rst | 12 +- src/greenlet/TPythonState.cpp | 9 +- .../tests/fail_issue_515_freethread_gc.py | 112 ++++++++++++++++++ src/greenlet/tests/test_gc.py | 12 ++ 4 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 src/greenlet/tests/fail_issue_515_freethread_gc.py diff --git a/CHANGES.rst b/CHANGES.rst index daacbefd..d11ebc30 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -5,7 +5,17 @@ 3.5.4 (unreleased) ================== -- Nothing changed yet. +- 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. A newly + started greenlet incorrectly inherited the parent thread state's + ``_PyCStackRef`` list head; because those nodes live on the parent + greenlet's C stack, they became dangling once the child overwrote + that stack region, and the free-threaded collector crashed + dereferencing them in ``gc_visit_thread_stacks``. A new greenlet now + starts with an empty C-stack-reference list, just like a brand-new + thread. See `issue 515 + `_. 3.5.3 (2026-06-26) diff --git a/src/greenlet/TPythonState.cpp b/src/greenlet/TPythonState.cpp index bdc0eef2..0f10c5ba 100644 --- a/src/greenlet/TPythonState.cpp +++ b/src/greenlet/TPythonState.cpp @@ -332,7 +332,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. diff --git a/src/greenlet/tests/fail_issue_515_freethread_gc.py b/src/greenlet/tests/fail_issue_515_freethread_gc.py new file mode 100644 index 00000000..42b11be7 --- /dev/null +++ b/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) diff --git a/src/greenlet/tests/test_gc.py b/src/greenlet/tests/test_gc.py index 340fdbe3..fa1110d2 100644 --- a/src/greenlet/tests/test_gc.py +++ b/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,17 @@ 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_crashing_deferred_object(self): if sys.version_info < (3, 15): self.skipTest("Test is 3.15+ only") From 980ee7a9ef448b04d684149b977040d7a748b3fa Mon Sep 17 00:00:00 2001 From: Dorian Hoxha Date: Mon, 6 Jul 2026 12:05:03 +0200 Subject: [PATCH 2/5] Visit a suspended greenlet's C-stack refs during GC (issue #515) Follow-up to the #515 fix. A greenlet that suspends while the interpreter is holding a _PyCStackRef (for example, mid attribute resolution) parks those deferred references on its C stack. The free-threaded collector only walks the running thread's list in gc_visit_thread_stacks(), so an object reachable only through a suspended greenlet's C-stack ref could be collected early and used after free once the greenlet resumes. greenlet can't just walk the saved list head from tp_traverse: those nodes live on the greenlet's C stack, which is relocated into a heap copy while suspended, so the head points into memory that now belongs to whichever greenlet is running. Instead, snapshot strong references to the held objects in operator<< (while the stack is still coherent), visit them from tp_traverse, and release them in operator>>. update_refs() derives gc_refs from Py_REFCNT, so the held incref is balanced by the traverse subtract. Strong references rather than _Py_VISIT_STACKREF because _PyGC_VisitStackRef is not exported before 3.15, and a raw array rather than a Python container because operator<< must not allocate a GC-tracked object mid-switch. The accompanying test pins a deferred-refcounted class through a metaclass __get__, switches away from inside it, drops every other reference and collects; the class survives only with the fix. --- CHANGES.rst | 11 +++ src/greenlet/TGreenlet.hpp | 17 ++++ src/greenlet/TPythonState.cpp | 86 +++++++++++++++++++ .../tests/fail_c_stack_refs_suspended_gc.py | 72 ++++++++++++++++ src/greenlet/tests/test_gc.py | 15 ++++ 5 files changed, 201 insertions(+) create mode 100644 src/greenlet/tests/fail_c_stack_refs_suspended_gc.py diff --git a/CHANGES.rst b/CHANGES.rst index d11ebc30..3c2b764b 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -17,6 +17,17 @@ thread. See `issue 515 `_. +- 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). Those nodes carry deferred references, and the + free-threaded collector only visits the running thread's list in + ``gc_visit_thread_stacks``, so an object reachable only through a + suspended greenlet's C-stack reference could be collected early and + used after free on resume. greenlet now snapshots those references + when a greenlet suspends and visits them from ``tp_traverse``. See + `issue 515 `_. + 3.5.3 (2026-06-26) ================== diff --git a/src/greenlet/TGreenlet.hpp b/src/greenlet/TGreenlet.hpp index 764d46f7..e50ae95a 100644 --- a/src/greenlet/TGreenlet.hpp +++ b/src/greenlet/TGreenlet.hpp @@ -139,6 +139,13 @@ 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. + PyObject** c_stack_ref_snapshot; + Py_ssize_t c_stack_ref_snapshot_len; #endif #elif GREENLET_PY312 int py_recursion_depth; @@ -170,9 +177,19 @@ 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<<). clear_c_stack_ref_snapshot() drops those references. + void capture_c_stack_refs(const PyThreadState* tstate) noexcept; + void clear_c_stack_ref_snapshot() noexcept; +#endif + public: PythonState(); + ~PythonState(); // You can use this for testing whether we have a frame // or not. It returns const so they can't modify it. const OwnedFrame& top_frame() const noexcept; diff --git a/src/greenlet/TPythonState.cpp b/src/greenlet/TPythonState.cpp index 0f10c5ba..368648b9 100644 --- a/src/greenlet/TPythonState.cpp +++ b/src/greenlet/TPythonState.cpp @@ -18,6 +18,8 @@ PythonState::PythonState() ,stackpointer(nullptr) #ifdef Py_GIL_DISABLED ,c_stack_refs(nullptr) + ,c_stack_ref_snapshot(nullptr) + ,c_stack_ref_snapshot_len(0) #endif #elif GREENLET_PY312 ,py_recursion_depth(0) @@ -92,6 +94,68 @@ PythonState::PythonState() #endif } +PythonState::~PythonState() +{ +#if GREENLET_PY314 && defined(Py_GIL_DISABLED) + this->clear_c_stack_ref_snapshot(); +#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. Ordinary strong + // references (not _Py_VISIT_STACKREF) because _PyGC_VisitStackRef isn't + // exported before 3.15, and a raw array rather than a Python container + // because operator<< must not allocate a GC-tracked object mid-switch. + // Reallocated from scratch each time; the list is empty at a typical + // switch, so this is usually just the count == 0 early return. + this->clear_c_stack_ref_snapshot(); + + const _PyCStackRef* const head = ((_PyThreadStateImpl*)tstate)->c_stack_refs; + Py_ssize_t count = 0; + for (const _PyCStackRef* node = head; node != nullptr; node = node->next) { + if (!PyStackRef_IsNullOrInt(node->ref)) { + count++; + } + } + if (count == 0) { + return; + } + PyObject** buf = (PyObject**)PyMem_Malloc(count * sizeof(PyObject*)); + if (buf == nullptr) { + // Out of memory mid-switch. Leaving the snapshot empty only forgoes the + // extra GC protection (i.e. the pre-fix behavior); we make nothing + // worse, and the switch will fail for the same reason a moment later. + return; + } + Py_ssize_t i = 0; + for (const _PyCStackRef* node = head; node != nullptr; node = node->next) { + if (!PyStackRef_IsNullOrInt(node->ref)) { + PyObject* obj = PyStackRef_AsPyObjectBorrow(node->ref); + Py_INCREF(obj); + buf[i++] = obj; + } + } + this->c_stack_ref_snapshot = buf; + this->c_stack_ref_snapshot_len = count; +} + +void PythonState::clear_c_stack_ref_snapshot() noexcept +{ + for (Py_ssize_t i = 0; i < this->c_stack_ref_snapshot_len; i++) { + Py_DECREF(this->c_stack_ref_snapshot[i]); + } + PyMem_Free(this->c_stack_ref_snapshot); + this->c_stack_ref_snapshot = nullptr; + this->c_stack_ref_snapshot_len = 0; +} +#endif + inline void PythonState::may_switch_away() noexcept { @@ -145,6 +209,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; @@ -252,6 +319,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->clear_c_stack_ref_snapshot(); #endif this->unexpose_frames(); #elif GREENLET_PY312 @@ -389,6 +460,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. c_stack_ref_snapshot_len is + // 0 while we're the running greenlet, so this is a no-op there. + for (Py_ssize_t i = 0; i < this->c_stack_ref_snapshot_len; i++) { + Py_VISIT(this->c_stack_ref_snapshot[i]); + } #endif // Note that we DO NOT visit ``delete_later``. Even if it's // non-null and we technically own a reference to it, its @@ -402,6 +485,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->clear_c_stack_ref_snapshot(); +#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. diff --git a/src/greenlet/tests/fail_c_stack_refs_suspended_gc.py b/src/greenlet/tests/fail_c_stack_refs_suspended_gc.py new file mode 100644 index 00000000..c2838c44 --- /dev/null +++ b/src/greenlet/tests/fail_c_stack_refs_suspended_gc.py @@ -0,0 +1,72 @@ +"""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. + 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) diff --git a/src/greenlet/tests/test_gc.py b/src/greenlet/tests/test_gc.py index fa1110d2..d725143a 100644 --- a/src/greenlet/tests/test_gc.py +++ b/src/greenlet/tests/test_gc.py @@ -96,6 +96,21 @@ def test_issue515_freethread_c_stack_refs(self): 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") From 39bf70c2cb8c8996ce26bc20e4161fd94c05d380 Mon Sep 17 00:00:00 2001 From: Dorian Hoxha Date: Mon, 6 Jul 2026 13:44:31 +0200 Subject: [PATCH 3/5] Hold the C-stack ref snapshot in a std::vector Lets the RAII wrapper own and release the references, replacing the hand-managed PyObject* array (manual incref/decref, a destructor, a length field). No behavior change. --- src/greenlet/TGreenlet.hpp | 8 ++-- src/greenlet/TPythonState.cpp | 74 ++++++++--------------------------- 2 files changed, 20 insertions(+), 62 deletions(-) diff --git a/src/greenlet/TGreenlet.hpp b/src/greenlet/TGreenlet.hpp index e50ae95a..0c63b467 100644 --- a/src/greenlet/TGreenlet.hpp +++ b/src/greenlet/TGreenlet.hpp @@ -8,6 +8,7 @@ #include #include +#include #include "greenlet_compiler_compat.hpp" #include "greenlet_refs.hpp" @@ -144,8 +145,7 @@ namespace greenlet // 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. - PyObject** c_stack_ref_snapshot; - Py_ssize_t c_stack_ref_snapshot_len; + std::vector c_stack_ref_snapshot; #endif #elif GREENLET_PY312 int py_recursion_depth; @@ -181,15 +181,13 @@ namespace greenlet // 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<<). clear_c_stack_ref_snapshot() drops those references. + // (operator<<). The snapshot is dropped by clear()ing the vector. void capture_c_stack_refs(const PyThreadState* tstate) noexcept; - void clear_c_stack_ref_snapshot() noexcept; #endif public: PythonState(); - ~PythonState(); // You can use this for testing whether we have a frame // or not. It returns const so they can't modify it. const OwnedFrame& top_frame() const noexcept; diff --git a/src/greenlet/TPythonState.cpp b/src/greenlet/TPythonState.cpp index 368648b9..4e968057 100644 --- a/src/greenlet/TPythonState.cpp +++ b/src/greenlet/TPythonState.cpp @@ -18,8 +18,6 @@ PythonState::PythonState() ,stackpointer(nullptr) #ifdef Py_GIL_DISABLED ,c_stack_refs(nullptr) - ,c_stack_ref_snapshot(nullptr) - ,c_stack_ref_snapshot_len(0) #endif #elif GREENLET_PY312 ,py_recursion_depth(0) @@ -94,13 +92,6 @@ PythonState::PythonState() #endif } -PythonState::~PythonState() -{ -#if GREENLET_PY314 && defined(Py_GIL_DISABLED) - this->clear_c_stack_ref_snapshot(); -#endif -} - #if GREENLET_PY314 && defined(Py_GIL_DISABLED) void PythonState::capture_c_stack_refs(const PyThreadState* tstate) noexcept { @@ -108,51 +99,20 @@ void PythonState::capture_c_stack_refs(const PyThreadState* tstate) noexcept // 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. Ordinary strong - // references (not _Py_VISIT_STACKREF) because _PyGC_VisitStackRef isn't - // exported before 3.15, and a raw array rather than a Python container - // because operator<< must not allocate a GC-tracked object mid-switch. - // Reallocated from scratch each time; the list is empty at a typical - // switch, so this is usually just the count == 0 early return. - this->clear_c_stack_ref_snapshot(); - - const _PyCStackRef* const head = ((_PyThreadStateImpl*)tstate)->c_stack_refs; - Py_ssize_t count = 0; - for (const _PyCStackRef* node = head; node != nullptr; node = node->next) { + // 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)) { - count++; + this->c_stack_ref_snapshot.push_back( + OwnedObject::owning(PyStackRef_AsPyObjectBorrow(node->ref))); } } - if (count == 0) { - return; - } - PyObject** buf = (PyObject**)PyMem_Malloc(count * sizeof(PyObject*)); - if (buf == nullptr) { - // Out of memory mid-switch. Leaving the snapshot empty only forgoes the - // extra GC protection (i.e. the pre-fix behavior); we make nothing - // worse, and the switch will fail for the same reason a moment later. - return; - } - Py_ssize_t i = 0; - for (const _PyCStackRef* node = head; node != nullptr; node = node->next) { - if (!PyStackRef_IsNullOrInt(node->ref)) { - PyObject* obj = PyStackRef_AsPyObjectBorrow(node->ref); - Py_INCREF(obj); - buf[i++] = obj; - } - } - this->c_stack_ref_snapshot = buf; - this->c_stack_ref_snapshot_len = count; -} - -void PythonState::clear_c_stack_ref_snapshot() noexcept -{ - for (Py_ssize_t i = 0; i < this->c_stack_ref_snapshot_len; i++) { - Py_DECREF(this->c_stack_ref_snapshot[i]); - } - PyMem_Free(this->c_stack_ref_snapshot); - this->c_stack_ref_snapshot = nullptr; - this->c_stack_ref_snapshot_len = 0; } #endif @@ -322,7 +282,7 @@ void PythonState::operator>>(PyThreadState *const tstate) noexcept // 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->clear_c_stack_ref_snapshot(); + this->c_stack_ref_snapshot.clear(); #endif this->unexpose_frames(); #elif GREENLET_PY312 @@ -467,10 +427,10 @@ int PythonState::tp_traverse(visitproc visit, void* arg, bool visit_top_frame) n // 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. c_stack_ref_snapshot_len is - // 0 while we're the running greenlet, so this is a no-op there. - for (Py_ssize_t i = 0; i < this->c_stack_ref_snapshot_len; i++) { - Py_VISIT(this->c_stack_ref_snapshot[i]); + // 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 @@ -486,7 +446,7 @@ void PythonState::tp_clear(bool own_top_frame) noexcept { PythonStateContext::tp_clear(); #if GREENLET_PY314 && defined(Py_GIL_DISABLED) - this->clear_c_stack_ref_snapshot(); + 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 From 78932dc67b6571a920751339cfd88056144e8116 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Tue, 21 Jul 2026 11:41:36 -0500 Subject: [PATCH 4/5] Change notes are for end users, they don't need to go into technical detail Interested readers can follow the link to the issue or PR. --- CHANGES.rst | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 3c2b764b..b0b99478 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -7,25 +7,14 @@ - 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. A newly - started greenlet incorrectly inherited the parent thread state's - ``_PyCStackRef`` list head; because those nodes live on the parent - greenlet's C stack, they became dangling once the child overwrote - that stack region, and the free-threaded collector crashed - dereferencing them in ``gc_visit_thread_stacks``. A new greenlet now - starts with an empty C-stack-reference list, just like a brand-new - thread. See `issue 515 + started from a non-empty C-stack-reference state is active. + See `issue 515 `_. - 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). Those nodes carry deferred references, and the - free-threaded collector only visits the running thread's list in - ``gc_visit_thread_stacks``, so an object reachable only through a - suspended greenlet's C-stack reference could be collected early and - used after free on resume. greenlet now snapshots those references - when a greenlet suspends and visits them from ``tp_traverse``. See + resolution). See `issue 515 `_. From 73a0a1aadf8d45db71754954d588d433fa826a2a Mon Sep 17 00:00:00 2001 From: Dorian Hoxha Date: Tue, 21 Jul 2026 23:08:47 +0200 Subject: [PATCH 5/5] Fix the suspended C-stack-ref GC test to catch its regression The descriptor protocol handed __get__ the class as its cls local, and on 3.15 greenlet traverses a suspended greenlet's frames, so that local kept the class alive and hid the bug. Delete cls/obj/objtype before switching so the class survives only through the C-stack ref the fix protects. --- src/greenlet/tests/fail_c_stack_refs_suspended_gc.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/greenlet/tests/fail_c_stack_refs_suspended_gc.py b/src/greenlet/tests/fail_c_stack_refs_suspended_gc.py index c2838c44..8783185d 100644 --- a/src/greenlet/tests/fail_c_stack_refs_suspended_gc.py +++ b/src/greenlet/tests/fail_c_stack_refs_suspended_gc.py @@ -36,6 +36,11 @@ def __get__(cls, obj, objtype=None): # 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