diff --git a/CHANGES.rst b/CHANGES.rst index 1940901..547c834 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -5,7 +5,54 @@ 3.2.6 (unreleased) ================== -- Nothing changed yet. +- Fix multiple crash paths during interpreter shutdown on **all Python + versions** (observed with uWSGI worker recycling on ARM64 and x86_64). + Two independent guards now protect all shutdown phases: + + 1. ``g_greenlet_shutting_down`` — an atexit handler registered at + module init (LIFO = runs first) sets this flag. Covers the atexit + phase of ``Py_FinalizeEx``, where ``Py_IsFinalizing()`` is still + ``False`` on all Python versions. + + 2. ``Py_IsFinalizing()`` — covers the GC collection and later phases + of ``Py_FinalizeEx``, where ``__del__`` methods and C++ destructors + run. A compatibility shim is provided for Python < 3.13 (where + only the private ``_Py_IsFinalizing()`` existed). + + These guards are checked in ``mod_getcurrent``, + ``PyGreenlet_GetCurrent``, ``GreenletChecker``, + ``MainGreenletExactChecker``, ``ContextExactChecker``, + ``clear_deleteme_list()``, ``ThreadState::~ThreadState()``, + ``_green_dealloc_kill_started_non_main_greenlet``, and + ``ThreadState_DestroyNoGIL::AddPendingCall``. + + Additional hardening: + + - ``clear_deleteme_list()`` uses ``std::swap`` (zero-allocation) + instead of copying the ``PythonAllocator``-backed vector. + - The ``deleteme`` vector uses ``std::allocator`` (system ``malloc``) + instead of ``PyMem_Malloc``. + - ``ThreadState`` uses ``std::malloc`` / ``std::free`` instead of + ``PyObject_Malloc``. + - ``clear_deleteme_list()`` preserves any pending Python exception + around its cleanup loop. + + Verified via TDD: tests fail on greenlet 3.3.2 (UNGUARDED) and pass + with the fix (GUARDED) across Python 3.10–3.14. + + This is distinct from the dealloc crash fixed in 3.2.5 + (`PR #495 + `_). + Backported from `PR #499 + `_ by Nicolas + Bouvrette. + +- Fix ``test_dealloc_catches_GreenletExit_throws_other`` to use + ``sys.unraisablehook`` instead of stderr capture, making it work + with both pytest and unittest runners. + +- Fix ``test_version`` to skip gracefully when the local setuptools + version does not support PEP 639 SPDX license format. 3.2.5 (2026-02-20) diff --git a/setup.py b/setup.py index c0e3c34..5569a86 100755 --- a/setup.py +++ b/setup.py @@ -225,7 +225,7 @@ def get_greenlet_version(): 'Documentation': 'https://greenlet.readthedocs.io/', 'Changes': 'https://greenlet.readthedocs.io/en/latest/changes.html', }, - license="MIT AND Python-2.0", + license="MIT AND PSF-2.0", license_files=[ 'LICENSE', 'LICENSE.PSF', diff --git a/src/greenlet/CObjects.cpp b/src/greenlet/CObjects.cpp index c135995..a5a9921 100644 --- a/src/greenlet/CObjects.cpp +++ b/src/greenlet/CObjects.cpp @@ -29,6 +29,9 @@ extern "C" { static PyGreenlet* PyGreenlet_GetCurrent(void) { + if (greenlet::IsShuttingDown()) { + return nullptr; + } return GET_THREAD_STATE().state().get_current().relinquish_ownership(); } diff --git a/src/greenlet/PyGreenlet.cpp b/src/greenlet/PyGreenlet.cpp index 64589de..43bdefb 100644 --- a/src/greenlet/PyGreenlet.cpp +++ b/src/greenlet/PyGreenlet.cpp @@ -61,9 +61,9 @@ green_new(PyTypeObject* type, PyObject* UNUSED(args), PyObject* UNUSED(kwds)) // can do things like cause other greenlets to get finalized. UserGreenlet* c = new UserGreenlet(o, GET_THREAD_STATE().state().borrow_current()); assert(Py_REFCNT(o) == 1); - // Also: This looks like a memory leak, but isn't. Constructing the - // C++ object assigns it to the pimpl pointer of the Python object (o); - // we'll need that later. + // Also: This looks like a memory leak, but isn't. + // Constructing the C++ object assigns it to the pimpl pointer + // of the Python object (o); we'll need that later. assert(c == o->pimpl); } return o; @@ -203,12 +203,10 @@ _green_dealloc_kill_started_non_main_greenlet(BorrowedGreenlet self) // // See: https://github.com/python-greenlet/greenlet/issues/411 // https://github.com/python-greenlet/greenlet/issues/351 -#if !GREENLET_PY311 - if (_Py_IsFinalizing()) { + if (greenlet::IsShuttingDown()) { self->murder_in_place(); return 1; } -#endif /* Hacks hacks hacks copied from instance_dealloc() */ /* Temporarily resurrect the greenlet. */ diff --git a/src/greenlet/PyModule.cpp b/src/greenlet/PyModule.cpp index 6adcb5c..7d070f4 100644 --- a/src/greenlet/PyModule.cpp +++ b/src/greenlet/PyModule.cpp @@ -17,6 +17,20 @@ using greenlet::ThreadState; # pragma clang diagnostic ignored "-Wunused-variable" #endif + +static PyObject* +_greenlet_atexit_callback(PyObject* UNUSED(self), PyObject* UNUSED(args)) +{ + greenlet::g_greenlet_shutting_down = 1; + Py_RETURN_NONE; +} + +static PyMethodDef _greenlet_atexit_method = { + "_greenlet_cleanup", _greenlet_atexit_callback, + METH_NOARGS, NULL +}; + + PyDoc_STRVAR(mod_getcurrent_doc, "getcurrent() -> greenlet\n" "\n" @@ -26,6 +40,9 @@ PyDoc_STRVAR(mod_getcurrent_doc, static PyObject* mod_getcurrent(PyObject* UNUSED(module)) { + if (greenlet::IsShuttingDown()) { + Py_RETURN_NONE; + } return GET_THREAD_STATE().state().get_current().relinquish_ownership_o(); } diff --git a/src/greenlet/TThreadState.hpp b/src/greenlet/TThreadState.hpp index cd97c84..eba1c90 100644 --- a/src/greenlet/TThreadState.hpp +++ b/src/greenlet/TThreadState.hpp @@ -1,6 +1,7 @@ #ifndef GREENLET_THREAD_STATE_HPP #define GREENLET_THREAD_STATE_HPP +#include #include #include @@ -22,6 +23,7 @@ using greenlet::refs::CreatedModule; using greenlet::refs::PyErrPieces; using greenlet::refs::NewReference; + namespace greenlet { /** * Thread-local state of greenlets. @@ -104,7 +106,13 @@ class ThreadState { /* Strong reference to the trace function, if any. */ OwnedObject tracefunc; - typedef std::vector > deleteme_t; + // Use std::allocator (malloc/free) instead of PythonAllocator + // (PyMem_Malloc) for the deleteme list. During Py_FinalizeEx on + // Python < 3.11, the PyObject_Malloc pool that holds ThreadState + // can be disrupted, corrupting any PythonAllocator-backed + // containers. Using std::allocator makes this vector independent + // of Python's allocator lifecycle. + typedef std::vector deleteme_t; /* A vector of raw PyGreenlet pointers representing things that need deleted when this thread is running. The vector owns the references, but you need to manually INCREF/DECREF as you use @@ -120,7 +128,6 @@ class ThreadState { static std::clock_t _clocks_used_doing_gc; static ImmortalString get_referrers_name; - static PythonAllocator allocator; G_NO_COPIES_OF_CLS(ThreadState); @@ -146,15 +153,23 @@ class ThreadState { public: - static void* operator new(size_t UNUSED(count)) + // Allocate ThreadState with malloc/free rather than Python's + // object allocator. ThreadState outlives many Python objects and + // must remain valid throughout Py_FinalizeEx. On Python < 3.11, + // PyObject_Malloc pools can be disrupted during early + // finalization, corrupting any C++ objects stored in them. + static void* operator new(size_t count) { - return ThreadState::allocator.allocate(1); + void* p = std::malloc(count); + if (!p) { + throw std::bad_alloc(); + } + return p; } static void operator delete(void* ptr) { - return ThreadState::allocator.deallocate(static_cast(ptr), - 1); + std::free(ptr); } static void init() @@ -283,33 +298,59 @@ class ThreadState { inline void clear_deleteme_list(const bool murder=false) { if (!this->deleteme.empty()) { - // It's possible we could add items to this list while - // running Python code if there's a thread switch, so we - // need to defensively copy it before that can happen. - deleteme_t copy = this->deleteme; - this->deleteme.clear(); // in case things come back on the list + // Move the list contents out with swap — a constant-time + // pointer exchange that never allocates. The previous + // code used a copy (deleteme_t copy = this->deleteme) + // which allocated through PythonAllocator / PyMem_Malloc; + // that could SIGSEGV during early Py_FinalizeEx on Python + // < 3.11 when the allocator is partially torn down. + deleteme_t copy; + std::swap(copy, this->deleteme); + + // During Py_FinalizeEx cleanup, the GC or atexit handlers + // may have already collected objects in this list, + // leaving dangling pointers. Attempting Py_DECREF on + // freed memory causes a SIGSEGV. g_greenlet_shutting_down + // covers the early atexit phase; Py_IsFinalizing() covers + // later phases. Thus, we deliberately leak. + if (greenlet::IsShuttingDown()) { + return; + } + + // Preserve any pending exception so that cleanup-triggered + // errors don't accidentally swallow an unrelated exception + // (e.g. one set by throw() before a switch). + PyErrPieces incoming_err; + for(deleteme_t::iterator it = copy.begin(), end = copy.end(); it != end; ++it ) { PyGreenlet* to_del = *it; if (murder) { - // Force each greenlet to appear dead; we can't raise an - // exception into it anymore anyway. to_del->pimpl->murder_in_place(); } - - // The only reference to these greenlets should be in - // this list, decreffing them should let them be - // deleted again, triggering calls to green_dealloc() - // in the correct thread (if we're not murdering). - // This may run arbitrary Python code and switch - // threads or greenlets! Py_DECREF(to_del); if (PyErr_Occurred()) { PyErr_WriteUnraisable(nullptr); PyErr_Clear(); } } + // Not worried about C++ exception safety here in terms of + // making sure we restore the error. Either we'll catch it + // above and establish the error from that exception + // (which, yes, might overwrite something from before we + // entered, but we're in an undefined situation at that + // point) or we won't catch it at all and will crash the + // process. + // + // As for Python exception safety, there's no chance we're + // overwriting an exception (from the loop) with no + // exception (captured NULLs before we entered the loop), + // because there CAN'T BE any exception from the loop --- + // we clear them. So we're either restoring a pre-existing + // exception, or leaving the exception unset (by restoring + // NULL). + incoming_err.PyErrRestore(); } } @@ -365,13 +406,12 @@ class ThreadState { // During interpreter finalization, Python APIs like // PyImport_ImportModule are unsafe (the import machinery may // be partially torn down). On Python < 3.11, perform only the - // minimal cleanup that is safe: clear our strong references so - // we don't leak, but skip the GC-based leak detection. + // minimal cleanup that is safe: clear our strong references + // so we don't leak, but skip the GC-based leak detection. // // Python 3.11+ restructured interpreter finalization so that // these APIs remain safe during shutdown. -#if !GREENLET_PY311 - if (_Py_IsFinalizing()) { + if (greenlet::IsShuttingDown()) { this->tracefunc.CLEAR(); if (this->current_greenlet) { this->current_greenlet->murder_in_place(); @@ -380,7 +420,6 @@ class ThreadState { this->main_greenlet.CLEAR(); return; } -#endif // We should not have an "origin" greenlet; that only exists // for the temporary time during a switch, which should not @@ -505,7 +544,6 @@ class ThreadState { }; ImmortalString ThreadState::get_referrers_name(nullptr); -PythonAllocator ThreadState::allocator; std::clock_t ThreadState::_clocks_used_doing_gc(0); diff --git a/src/greenlet/TThreadStateDestroy.cpp b/src/greenlet/TThreadStateDestroy.cpp index 449b788..8bbf75b 100644 --- a/src/greenlet/TThreadStateDestroy.cpp +++ b/src/greenlet/TThreadStateDestroy.cpp @@ -170,18 +170,15 @@ struct ThreadState_DestroyNoGIL static int AddPendingCall(int (*func)(void*), void* arg) { - // If the interpreter is in the middle of finalizing, we can't add a - // pending call. Trying to do so will end up in a SIGSEGV, as - // Py_AddPendingCall will not be able to get the interpreter and will - // try to dereference a NULL pointer. It's possible this can still - // segfault if we happen to get context switched, and maybe we should - // just always implement our own AddPendingCall, but I'd like to see if - // this works first -#if GREENLET_PY313 - if (Py_IsFinalizing()) { -#else - if (_Py_IsFinalizing()) { -#endif + // If the interpreter is in the middle of finalizing, we can't + // add a pending call. Trying to do so will end up in a + // SIGSEGV, as Py_AddPendingCall will not be able to get the + // interpreter and will try to dereference a NULL pointer. + // It's possible this can still segfault if we happen to get + // context switched, and maybe we should just always implement + // our own AddPendingCall, but I'd like to see if this works + // first + if (greenlet::IsShuttingDown()) { #ifdef GREENLET_DEBUG // No need to log in the general case. Yes, we'll leak, // but we're shutting down so it should be ok. diff --git a/src/greenlet/greenlet.cpp b/src/greenlet/greenlet.cpp index e8d92a0..27d993c 100644 --- a/src/greenlet/greenlet.cpp +++ b/src/greenlet/greenlet.cpp @@ -232,6 +232,26 @@ greenlet_internal_mod_init() noexcept OwnedObject clocks_per_sec = OwnedObject::consuming(PyLong_FromSsize_t(CLOCKS_PER_SEC)); m.PyAddObject("CLOCKS_PER_SEC", clocks_per_sec); + // Register an atexit handler that sets + // g_greenlet_shutting_down. Python's atexit is LIFO: + // registered last = called first. By registering here (at + // import time, after most other libraries), our handler runs + // before their cleanup code, which may try to call + // greenlet.getcurrent() on objects whose type has been + // invalidated. _Py_IsFinalizing() alone is insufficient on + // ALL Python versions because it is only set AFTER atexit + // handlers complete inside Py_FinalizeEx. + { + extern PyMethodDef _greenlet_atexit_method; + NewReference atexit_mod(Require(PyImport_ImportModule("atexit"))); + OwnedObject register_fn = atexit_mod.PyRequireAttr("register"); + NewReference callback(Require( + PyCFunction_New(&_greenlet_atexit_method, NULL))); + NewReference args(Require(PyTuple_Pack(1, callback.borrow()))); + NewReference result(Require( + PyObject_Call(register_fn.borrow(), args.borrow(), NULL))); + } + /* also publish module-level data as attributes of the greentype. */ // XXX: This is weird, and enables a strange pattern of // confusing the class greenlet with the module greenlet; with diff --git a/src/greenlet/greenlet_cpython_compat.hpp b/src/greenlet/greenlet_cpython_compat.hpp index a3b3850..c817a87 100644 --- a/src/greenlet/greenlet_cpython_compat.hpp +++ b/src/greenlet/greenlet_cpython_compat.hpp @@ -147,4 +147,12 @@ static inline void PyThreadState_LeaveTracing(PyThreadState *tstate) # define Py_C_RECURSION_LIMIT C_RECURSION_LIMIT #endif +// Py_IsFinalizing() became a public API in Python 3.13. Map it to the +// private _Py_IsFinalizing() on older versions so all call sites can +// use the standard name. Remove this once greenlet drops support for +// Python < 3.13. +#if !GREENLET_PY313 +# define Py_IsFinalizing() _Py_IsFinalizing() +#endif + #endif /* GREENLET_CPYTHON_COMPAT_H */ diff --git a/src/greenlet/greenlet_internal.hpp b/src/greenlet/greenlet_internal.hpp index f2b15d5..e9d03db 100644 --- a/src/greenlet/greenlet_internal.hpp +++ b/src/greenlet/greenlet_internal.hpp @@ -46,6 +46,9 @@ greenlet::refs::MainGreenletExactChecker(void *p) if (!p) { return; } + if (greenlet::IsShuttingDown()) { + return; + } // We control the class of the main greenlet exactly. if (Py_TYPE(p) != &PyGreenlet_Type) { std::string err("MainGreenlet: Expected exactly a greenlet, not a "); diff --git a/src/greenlet/greenlet_refs.hpp b/src/greenlet/greenlet_refs.hpp index b7e5e3f..64f07e2 100644 --- a/src/greenlet/greenlet_refs.hpp +++ b/src/greenlet/greenlet_refs.hpp @@ -27,6 +27,24 @@ using std::endl; namespace greenlet { class Greenlet; + // _Py_IsFinalizing() is only set AFTER atexit handlers complete + // inside Py_FinalizeEx on ALL Python versions (including 3.11+). + // Code running in atexit handlers (e.g. uWSGI plugin cleanup + // calling Py_FinalizeEx, New Relic agent shutdown) can still call + // greenlet.getcurrent(), but by that time type objects or + // internal state may have been invalidated. This flag is set by + // an atexit handler registered at module init (LIFO = runs + // first). + // + // Because this is only set from an atexit handler, by which point + // we're single threaded, there should be no need to make it std::atomic. + static int g_greenlet_shutting_down; + + static inline bool + IsShuttingDown() + { + return greenlet::g_greenlet_shutting_down || Py_IsFinalizing(); + } namespace refs { @@ -49,6 +67,9 @@ namespace greenlet if (!p) { return; } + if (IsShuttingDown()) { + return; + } PyTypeObject* typ = Py_TYPE(p); // fast, common path. (PyObject_TypeCheck is a macro or @@ -102,6 +123,9 @@ namespace greenlet if (!p) { return; } + if (IsShuttingDown()) { + return; + } if (!PyContext_CheckExact(p)) { throw TypeError( "greenlet context must be a contextvars.Context or None" diff --git a/src/greenlet/tests/test_greenlet.py b/src/greenlet/tests/test_greenlet.py index 1fa4bd1..d52b05e 100644 --- a/src/greenlet/tests/test_greenlet.py +++ b/src/greenlet/tests/test_greenlet.py @@ -206,22 +206,18 @@ def run(): g = RawGreenlet(run) g.switch() - # Destroying the only reference to the greenlet causes it - # to get GreenletExit; when it in turn raises, even though we're the parent - # we don't get the exception, it just gets printed. - # When we run on 3.8 only, we can use sys.unraisablehook - oldstderr = sys.stderr - from io import StringIO - stderr = sys.stderr = StringIO() + unraisable_events = [] + old_hook = sys.unraisablehook + def _capture(unraisable): + unraisable_events.append(unraisable) + sys.unraisablehook = _capture try: del g finally: - sys.stderr = oldstderr + sys.unraisablehook = old_hook - v = stderr.getvalue() - self.assertIn("Exception", v) - self.assertIn('ignored', v) - self.assertIn("SomeError", v) + self.assertEqual(len(unraisable_events), 1) + self.assertIsInstance(unraisable_events[0].exc_value, SomeError) @unittest.skipIf( diff --git a/src/greenlet/tests/test_interpreter_shutdown.py b/src/greenlet/tests/test_interpreter_shutdown.py index 37afc52..7032d5b 100644 --- a/src/greenlet/tests/test_interpreter_shutdown.py +++ b/src/greenlet/tests/test_interpreter_shutdown.py @@ -2,20 +2,31 @@ """ Tests for greenlet behavior during interpreter shutdown (Py_FinalizeEx). -Prior to the safe finalization fix, active greenlets being deallocated -during interpreter shutdown could trigger SIGSEGV or SIGABRT on Python -< 3.11, because green_dealloc attempted to throw GreenletExit via -g_switch() into a partially-torn-down interpreter. - -The fix adds _Py_IsFinalizing() guards (on Python < 3.11 only) that -call murder_in_place() instead of g_switch() when the interpreter is -shutting down, avoiding the crash at the cost of not running cleanup -code inside the greenlet. - -These tests verify: - 1. No crashes on ANY Python version (the core safety guarantee). - 2. GreenletExit cleanup code runs correctly during normal thread exit - (the standard production path, e.g. uWSGI worker threads). +During interpreter shutdown, several greenlet code paths can access +partially-destroyed Python state, leading to SIGSEGV. Two independent +guards protect against this on ALL Python versions: + + 1. g_greenlet_shutting_down — set by an atexit handler registered at + greenlet import time (LIFO = runs before other cleanup). Covers + the atexit phase of Py_FinalizeEx, where _Py_IsFinalizing() is + still False on all Python versions. + + 2. Py_IsFinalizing() — covers the GC collection and later phases of + Py_FinalizeEx, where __del__ methods and destructor code run. + +These tests are organized into four groups: + + A. Core safety (smoke): no crashes with active greenlets at shutdown. + B. Cleanup semantics: GreenletExit / finally still works during + normal thread exit (the standard production path). + C. Atexit "still works" tests: getcurrent() / greenlet construction + during atexit handlers registered AFTER greenlet import (i.e. + BEFORE greenlet's cleanup handler in LIFO order) must return + valid objects — verifies the guards don't over-block. + D. TDD-certified regression tests: getcurrent() must return None + when called AFTER greenlet's cleanup (GC finalization phase + or late atexit phase). These tests fail on greenlet 3.3.2 + and pass with the fix across Python 3.10-3.14. """ import sys import subprocess @@ -25,7 +36,7 @@ from greenlet.tests import TestCase -class TestInterpreterShutdown(TestCase): +class TestInterpreterShutdown(TestCase): # pylint:disable=too-many-public-methods def _run_shutdown_script(self, script_body): """ @@ -43,7 +54,7 @@ def _run_shutdown_script(self, script_body): return result.returncode, result.stdout, result.stderr # ----------------------------------------------------------------- - # Core safety tests: no crashes on any Python version + # Group A: Core safety — no crashes with active greenlets at exit # ----------------------------------------------------------------- def test_active_greenlet_at_shutdown_no_crash(self): @@ -151,24 +162,13 @@ def greenlet_func(): self.assertIn("OK: 3 threaded greenlets at shutdown", stdout) # ----------------------------------------------------------------- - # Cleanup semantics tests + # Group B: Cleanup semantics — thread exit # ----------------------------------------------------------------- # - # Note on behavioral testing during interpreter shutdown: - # - # During Py_FinalizeEx, sys.stdout is set to None early, making - # print() a no-op. More importantly, an active greenlet in the - # module-level scope interferes with module dict clearing — the - # greenlet's dealloc path (which temporarily resurrects the object - # and performs a stack switch via g_switch) prevents reliable - # observation of cleanup behavior. - # - # The production crash (SIGSEGV/SIGABRT) occurs during thread-state - # cleanup in Py_FinalizeEx, not during module dict clearing. Our - # _Py_IsFinalizing() guard in _green_dealloc_kill_started_non_main_ - # greenlet targets that path. The safety tests above verify that no - # crashes occur; the tests below verify that greenlet cleanup works - # correctly during normal thread exit (the most common code path). + # These tests verify that GreenletExit / try-finally still work + # correctly during normal thread exit (the standard production + # path, e.g. uWSGI worker threads finishing a request). This is + # NOT interpreter shutdown; the guards do not fire here. def test_greenlet_cleanup_during_thread_exit(self): """ @@ -315,6 +315,508 @@ def worker(): self.assertEqual(rc, 0, f"Process crashed (rc={rc}):\n{stdout}{stderr}") self.assertIn("OK: greenlet with active exception at shutdown", stdout) + # ----------------------------------------------------------------- + # Group C: getcurrent() / construction / gettrace() / settrace() + # during atexit — registered AFTER greenlet import + # + # These atexit handlers are registered AFTER ``import greenlet``, + # so they run BEFORE greenlet's own cleanup handler (LIFO). At + # this point g_greenlet_shutting_down is still 0 and + # _Py_IsFinalizing() is False, so getcurrent() must still return + # a valid greenlet object. These tests guard against the fix + # being too aggressive (over-blocking getcurrent early). + # ----------------------------------------------------------------- + + def test_getcurrent_during_atexit_no_crash(self): + """ + getcurrent() in an atexit handler registered AFTER greenlet + import must return a valid greenlet (not None), because LIFO + ordering means this handler runs BEFORE greenlet's cleanup. + """ + rc, stdout, stderr = self._run_shutdown_script("""\ + import atexit + import greenlet + + def call_getcurrent_at_exit(): + try: + g = greenlet.getcurrent() + if g is not None and type(g).__name__ == 'greenlet': + print(f"OK: getcurrent returned valid greenlet") + elif g is None: + print("FAIL: getcurrent returned None (over-blocked)") + else: + print(f"FAIL: unexpected {g!r}") + except Exception as e: + print(f"OK: getcurrent raised {type(e).__name__}: {e}") + + atexit.register(call_getcurrent_at_exit) + print("OK: atexit registered") + """) + self.assertEqual(rc, 0, f"Process crashed (rc={rc}):\n{stdout}{stderr}") + self.assertIn("OK: atexit registered", stdout) + self.assertIn("OK: getcurrent returned valid greenlet", stdout, + "getcurrent() should return a valid greenlet when called " + "before greenlet's cleanup handler (LIFO ordering)") + + def test_gettrace_during_atexit_no_crash(self): + """ + Calling greenlet.gettrace() during atexit must not crash. + """ + rc, stdout, stderr = self._run_shutdown_script("""\ + import atexit + import greenlet + + def check_at_exit(): + try: + result = greenlet.gettrace() + print(f"OK: gettrace returned {result!r}") + except Exception as e: + print(f"OK: gettrace raised {type(e).__name__}: {e}") + + atexit.register(check_at_exit) + print("OK: registered") + """) + self.assertEqual(rc, 0, f"Process crashed (rc={rc}):\n{stdout}{stderr}") + self.assertIn("OK: registered", stdout) + + def test_settrace_during_atexit_no_crash(self): + """ + Calling greenlet.settrace() during atexit must not crash. + """ + rc, stdout, stderr = self._run_shutdown_script("""\ + import atexit + import greenlet + + def check_at_exit(): + try: + greenlet.settrace(lambda *args: None) + print("OK: settrace succeeded") + except Exception as e: + print(f"OK: settrace raised {type(e).__name__}: {e}") + + atexit.register(check_at_exit) + print("OK: registered") + """) + self.assertEqual(rc, 0, f"Process crashed (rc={rc}):\n{stdout}{stderr}") + self.assertIn("OK: registered", stdout) + + def test_getcurrent_with_active_greenlets_during_atexit(self): + """ + getcurrent() during atexit (registered after import) with active + greenlets must still return a valid greenlet, since LIFO means + this runs before greenlet's cleanup. + """ + rc, stdout, stderr = self._run_shutdown_script("""\ + import atexit + import greenlet + + def worker(): + greenlet.getcurrent().parent.switch("ready") + + greenlets = [] + for i in range(5): + g = greenlet.greenlet(worker) + result = g.switch() + greenlets.append(g) + + def check_at_exit(): + try: + g = greenlet.getcurrent() + if g is not None and type(g).__name__ == 'greenlet': + print(f"OK: getcurrent returned valid greenlet") + elif g is None: + print("FAIL: getcurrent returned None (over-blocked)") + else: + print(f"FAIL: unexpected {g!r}") + except Exception as e: + print(f"OK: getcurrent raised {type(e).__name__}: {e}") + + atexit.register(check_at_exit) + print(f"OK: {len(greenlets)} active greenlets, atexit registered") + """) + self.assertEqual(rc, 0, f"Process crashed (rc={rc}):\n{stdout}{stderr}") + self.assertIn("OK: 5 active greenlets, atexit registered", stdout) + self.assertIn("OK: getcurrent returned valid greenlet", stdout, + "getcurrent() should return a valid greenlet when called " + "before greenlet's cleanup handler (LIFO ordering)") + + def test_greenlet_construction_during_atexit_no_crash(self): + """ + Constructing a new greenlet during atexit (registered after + import) must succeed, since this runs before greenlet's cleanup. + """ + rc, stdout, stderr = self._run_shutdown_script("""\ + import atexit + import greenlet + + def create_greenlets_at_exit(): + try: + def noop(): + pass + g = greenlet.greenlet(noop) + if g is not None: + print(f"OK: created greenlet successfully") + else: + print("FAIL: greenlet() returned None") + except Exception as e: + print(f"OK: construction raised {type(e).__name__}: {e}") + + atexit.register(create_greenlets_at_exit) + print("OK: atexit registered") + """) + self.assertEqual(rc, 0, f"Process crashed (rc={rc}):\n{stdout}{stderr}") + self.assertIn("OK: atexit registered", stdout) + self.assertIn("OK: created greenlet successfully", stdout) + + def test_greenlet_construction_with_active_greenlets_during_atexit(self): + """ + Constructing new greenlets during atexit when other active + greenlets already exist (maximizes the chance of a non-empty + deleteme list). + """ + rc, stdout, stderr = self._run_shutdown_script("""\ + import atexit + import greenlet + + def worker(): + greenlet.getcurrent().parent.switch("ready") + + greenlets = [] + for i in range(10): + g = greenlet.greenlet(worker) + g.switch() + greenlets.append(g) + + def create_at_exit(): + try: + new_greenlets = [] + for i in range(5): + g = greenlet.greenlet(lambda: None) + new_greenlets.append(g) + print(f"OK: created {len(new_greenlets)} greenlets at exit") + except Exception as e: + print(f"OK: raised {type(e).__name__}: {e}") + + atexit.register(create_at_exit) + print(f"OK: {len(greenlets)} active greenlets, atexit registered") + """) + self.assertEqual(rc, 0, f"Process crashed (rc={rc}):\n{stdout}{stderr}") + self.assertIn("OK: 10 active greenlets, atexit registered", stdout) + + def test_greenlet_construction_with_cross_thread_deleteme_during_atexit(self): + """ + Create greenlets in a worker thread, transfer them to the main + thread, then drop them — populating the deleteme list. Then + construct a new greenlet during atexit. On Python < 3.11 + clear_deleteme_list() could previously crash if the + PythonAllocator vector copy failed during early Py_FinalizeEx; + using std::swap eliminates that allocation. + """ + rc, stdout, stderr = self._run_shutdown_script("""\ + import atexit + import greenlet + import threading + + cross_thread_refs = [] + + def thread_worker(): + # Create greenlets in this thread + def gl_body(): + greenlet.getcurrent().parent.switch("ready") + for _ in range(20): + g = greenlet.greenlet(gl_body) + g.switch() + cross_thread_refs.append(g) + + t = threading.Thread(target=thread_worker) + t.start() + t.join() + + # Dropping these references in the main thread + # causes them to be added to the main thread's + # deleteme list (deferred cross-thread dealloc). + cross_thread_refs.clear() + + def create_at_exit(): + try: + g = greenlet.greenlet(lambda: None) + print(f"OK: created greenlet at exit {g!r}") + except Exception as e: + print(f"OK: raised {type(e).__name__}: {e}") + + atexit.register(create_at_exit) + print("OK: cross-thread setup done, atexit registered") + """) + self.assertEqual(rc, 0, f"Process crashed (rc={rc}):\n{stdout}{stderr}") + self.assertIn("OK: cross-thread setup done, atexit registered", stdout) + + + # ----------------------------------------------------------------- + # Group D.1: TDD-certified — getcurrent() during GC finalization + # + # These tests use gc.disable() + reference cycles to force __del__ + # to run during Py_FinalizeEx's GC pass, where Py_IsFinalizing() + # is True. Without the fix, getcurrent() returns a live greenlet + # (unguarded); with the fix, it returns None. + # + # TDD verification (greenlet 3.3.2 = RED, patched = GREEN): + # Python 3.10: RED (UNGUARDED) → GREEN (GUARDED) + # Python 3.11: RED (UNGUARDED) → GREEN (GUARDED) + # Python 3.12: RED (UNGUARDED) → GREEN (GUARDED) + # Python 3.13: RED (UNGUARDED) → GREEN (GUARDED) + # Python 3.14: RED (UNGUARDED) → GREEN (GUARDED) + # ----------------------------------------------------------------- + + def test_getcurrent_returns_none_during_gc_finalization(self): + """ + greenlet.getcurrent() must return None when called from a + __del__ method during Py_FinalizeEx's GC collection pass. + + On Python >= 3.11, _Py_IsFinalizing() is True during this + phase. Without the Py_IsFinalizing() guard in mod_getcurrent, + this would return a greenlet — the same unguarded code path + that leads to SIGSEGV in production (uWSGI worker recycling). + """ + rc, stdout, stderr = self._run_shutdown_script("""\ + import gc + import os + import greenlet + + gc.disable() + + class CleanupChecker: + def __del__(self): + try: + cur = greenlet.getcurrent() + if cur is None: + os.write(1, b"GUARDED: getcurrent=None\\n") + else: + os.write(1, b"UNGUARDED: getcurrent=" + + type(cur).__name__.encode() + b"\\n") + except Exception as e: + os.write(1, b"EXCEPTION: " + str(e).encode() + b"\\n") + + # Reference cycle: only collected during Py_FinalizeEx GC pass + a = CleanupChecker() + b = {"ref": a} + a._cycle = b + del a, b + + print("OK: deferred cycle created") + """) + self.assertEqual(rc, 0, f"Process crashed (rc={rc}):\n{stdout}{stderr}") + self.assertIn("OK: deferred cycle created", stdout) + self.assertIn("GUARDED: getcurrent=None", stdout, + "getcurrent() must return None during GC finalization; " + "returned a live object instead (missing Py_IsFinalizing guard)") + + def test_getcurrent_returns_none_during_gc_finalization_with_active_greenlets(self): + """ + Same as above but with active greenlets at shutdown, which + increases the amount of C++ destructor work during finalization. + """ + rc, stdout, stderr = self._run_shutdown_script("""\ + import gc + import os + import greenlet + + gc.disable() + + class CleanupChecker: + def __del__(self): + try: + cur = greenlet.getcurrent() + if cur is None: + os.write(1, b"GUARDED: getcurrent=None\\n") + else: + os.write(1, b"UNGUARDED: getcurrent=" + + type(cur).__name__.encode() + b"\\n") + except Exception as e: + os.write(1, b"EXCEPTION: " + str(e).encode() + b"\\n") + + # Create active greenlets + greenlets = [] + for _ in range(10): + def worker(): + greenlet.getcurrent().parent.switch("suspended") + g = greenlet.greenlet(worker) + g.switch() + greenlets.append(g) + + # Reference cycle deferred to Py_FinalizeEx + a = CleanupChecker() + b = {"ref": a} + a._cycle = b + del a, b + + print(f"OK: {len(greenlets)} active greenlets, cycle deferred") + """) + self.assertEqual(rc, 0, f"Process crashed (rc={rc}):\n{stdout}{stderr}") + self.assertIn("OK: 10 active greenlets, cycle deferred", stdout) + self.assertIn("GUARDED: getcurrent=None", stdout, + "getcurrent() must return None during GC finalization; " + "returned a live object instead (missing Py_IsFinalizing guard)") + + def test_getcurrent_returns_none_during_gc_finalization_cross_thread(self): + """ + Combines cross-thread greenlet deallocation (deleteme list) + with the GC finalization check. This simulates the production + scenario where uWSGI worker threads create greenlets that are + transferred to the main thread, then cleaned up during + Py_FinalizeEx. + """ + rc, stdout, stderr = self._run_shutdown_script("""\ + import gc + import os + import threading + import greenlet + + gc.disable() + + class CleanupChecker: + def __del__(self): + try: + cur = greenlet.getcurrent() + if cur is None: + os.write(1, b"GUARDED: getcurrent=None\\n") + else: + os.write(1, b"UNGUARDED: getcurrent=" + + type(cur).__name__.encode() + b"\\n") + except Exception as e: + os.write(1, b"EXCEPTION: " + str(e).encode() + b"\\n") + + # Create cross-thread greenlet references + cross_refs = [] + def thread_fn(): + for _ in range(20): + def body(): + greenlet.getcurrent().parent.switch("x") + g = greenlet.greenlet(body) + g.switch() + cross_refs.append(g) + t = threading.Thread(target=thread_fn) + t.start() + t.join() + cross_refs.clear() + + # Reference cycle deferred to Py_FinalizeEx + a = CleanupChecker() + b = {"ref": a} + a._cycle = b + del a, b + + print("OK: cross-thread cleanup + cycle deferred") + """) + self.assertEqual(rc, 0, f"Process crashed (rc={rc}):\n{stdout}{stderr}") + self.assertIn("OK: cross-thread cleanup + cycle deferred", stdout) + self.assertIn("GUARDED: getcurrent=None", stdout, + "getcurrent() must return None during GC finalization; " + "returned a live object instead (missing Py_IsFinalizing guard)") + + + # ----------------------------------------------------------------- + # Group D.2: TDD-certified — getcurrent() during atexit phase + # + # These tests register the checker BEFORE importing greenlet. + # Python's atexit is LIFO, so greenlet's handler (registered at + # import) runs FIRST and sets g_greenlet_shutting_down=1; then + # the checker runs SECOND and observes getcurrent() → None. + # + # This covers the atexit phase where _Py_IsFinalizing() is still + # False on ALL Python versions — the exact window that causes + # SIGSEGV in production (uWSGI worker recycling → Py_FinalizeEx). + # + # TDD verification (greenlet 3.3.2 = RED, patched = GREEN): + # Python 3.10: RED (UNGUARDED) → GREEN (GUARDED) + # Python 3.11: RED (UNGUARDED) → GREEN (GUARDED) + # Python 3.12: RED (UNGUARDED) → GREEN (GUARDED) + # Python 3.13: RED (UNGUARDED) → GREEN (GUARDED) + # Python 3.14: RED (UNGUARDED) → GREEN (GUARDED) + # ----------------------------------------------------------------- + + def test_getcurrent_returns_none_during_atexit_phase(self): + """ + greenlet.getcurrent() must return None when called from an + atexit handler that runs AFTER greenlet's own atexit handler. + + This tests the g_greenlet_shutting_down flag, which is needed + because _Py_IsFinalizing() is still False during the atexit + phase on ALL Python versions. Without g_greenlet_shutting_down, + getcurrent() proceeds unguarded into partially-torn-down state. + """ + rc, stdout, stderr = self._run_shutdown_script("""\ + import atexit + import os + + def late_checker(): + try: + import greenlet + cur = greenlet.getcurrent() + if cur is None: + os.write(1, b"GUARDED: getcurrent=None\\n") + else: + os.write(1, b"UNGUARDED: getcurrent=" + + type(cur).__name__.encode() + b"\\n") + except Exception as e: + os.write(1, b"EXCEPTION: " + str(e).encode() + b"\\n") + + # Register BEFORE importing greenlet. LIFO order: + # greenlet's handler (registered at import) runs FIRST, + # late_checker runs SECOND — seeing the flag already set. + atexit.register(late_checker) + + import greenlet + print("OK: atexit registered before greenlet import") + """) + self.assertEqual(rc, 0, f"Process crashed (rc={rc}):\n{stdout}{stderr}") + self.assertIn("OK: atexit registered before greenlet import", stdout) + self.assertIn("GUARDED: getcurrent=None", stdout, + "getcurrent() must return None during atexit phase; " + "returned a live object instead (missing " + "g_greenlet_shutting_down atexit handler)") + + def test_getcurrent_returns_none_during_atexit_phase_with_active_greenlets(self): + """ + Same as above but with active greenlets, ensuring the atexit + guard works even when there is greenlet state to clean up. + """ + rc, stdout, stderr = self._run_shutdown_script("""\ + import atexit + import os + + def late_checker(): + try: + import greenlet + cur = greenlet.getcurrent() + if cur is None: + os.write(1, b"GUARDED: getcurrent=None\\n") + else: + os.write(1, b"UNGUARDED: getcurrent=" + + type(cur).__name__.encode() + b"\\n") + except Exception as e: + os.write(1, b"EXCEPTION: " + str(e).encode() + b"\\n") + + atexit.register(late_checker) + + import greenlet + + greenlets = [] + for _ in range(10): + def worker(): + greenlet.getcurrent().parent.switch("parked") + g = greenlet.greenlet(worker) + g.switch() + greenlets.append(g) + + print(f"OK: {len(greenlets)} active greenlets, atexit registered") + """) + self.assertEqual(rc, 0, f"Process crashed (rc={rc}):\n{stdout}{stderr}") + self.assertIn("OK: 10 active greenlets, atexit registered", stdout) + self.assertIn("GUARDED: getcurrent=None", stdout, + "getcurrent() must return None during atexit phase; " + "returned a live object instead (missing " + "g_greenlet_shutting_down atexit handler)") + if __name__ == '__main__': unittest.main() diff --git a/src/greenlet/tests/test_leaks.py b/src/greenlet/tests/test_leaks.py index e09da7d..ed157dc 100644 --- a/src/greenlet/tests/test_leaks.py +++ b/src/greenlet/tests/test_leaks.py @@ -15,6 +15,7 @@ import greenlet from . import TestCase from . import PY314 +from . import WIN from . import RUNNING_ON_FREETHREAD_BUILD from .leakcheck import fails_leakcheck from .leakcheck import ignores_leakcheck @@ -439,7 +440,15 @@ def __call__(self): self.wait_for_pending_cleanups() uss_after = self.get_process_uss() - self.assertLessEqual(uss_after, uss_before, "after attempts %d" % (count,)) + # On Windows, USS can fluctuate by tens of KB between measurements + # due to working set trimming, page table updates, etc. Allow a + # small tolerance so OS-level noise doesn't cause false failures. + # Real leaks produce MBs of growth (each iteration creates 20k + # greenlets), so 512 KB is well below the detection threshold for + # genuine issues. + tolerance = 512 * 1024 if WIN else 0 + self.assertLessEqual(uss_after, uss_before + tolerance, + "after attempts %d" % (count,)) @ignores_leakcheck # Because we're just trying to track raw memory, not objects, and running diff --git a/src/greenlet/tests/test_version.py b/src/greenlet/tests/test_version.py index 96c17cf..2778b63 100755 --- a/src/greenlet/tests/test_version.py +++ b/src/greenlet/tests/test_version.py @@ -38,4 +38,10 @@ def find_dominating_file(name): with os.popen(invoke_setup) as f: sversion = f.read().strip() + if not sversion or not sversion[0].isdigit(): + self.skipTest( + "setup.py --version did not return a version string " + "(likely a setuptools compatibility issue): " + + repr(sversion[:80]) + ) self.assertEqual(sversion, greenlet.__version__)