Fix crash in getcurrent()/greenlet construction during early Py_FinalizeEx#499
Open
nbouvrette wants to merge 9 commits intopython-greenlet:masterfrom
Open
Fix crash in getcurrent()/greenlet construction during early Py_FinalizeEx#499nbouvrette wants to merge 9 commits intopython-greenlet:masterfrom
nbouvrette wants to merge 9 commits intopython-greenlet:masterfrom
Conversation
While regression-testing greenlet 3.2.5 (backport of PR python-greenlet#495) against Python 3.9.7 under uWSGI, we discovered a second, independent crash path that was NOT fixed by PR python-greenlet#495. Root cause ---------- On Python < 3.11, Py_FinalizeEx calls call_py_exitfuncs() BEFORE setting _PyRuntimeState.finalizing (which backs _Py_IsFinalizing()). If any exit function — or code triggered by one — calls greenlet.getcurrent(), greenlet.greenlet(...), or the C API PyGreenlet_GetCurrent(), the non-const get_current()/borrow_current() methods run clear_deleteme_list(). That helper copies a std::vector through PythonAllocator (PyMem_Malloc), and during this early finalization phase the allocator state can be partially torn down, causing a SIGSEGV. Fix --- Add ThreadStateCreator::readonly_state(), which returns a const ThreadState& and therefore selects the const overloads of get_current() / borrow_current() — these simply return the current greenlet pointer without touching the deleteme list. The deleteme cleanup is safely deferred to the next greenlet switch or thread-state teardown. Because the fix removes a side-effect from simple accessors, it is applied unconditionally (all Python versions), not just < 3.11. Changed files: - TThreadStateCreator.hpp: new readonly_state() method - TThreadState.hpp: new const borrow_current() overload - PyModule.cpp: mod_getcurrent uses readonly_state() - CObjects.cpp: PyGreenlet_GetCurrent uses readonly_state() - PyGreenlet.cpp: green_new uses readonly_state() - PyGreenletUnswitchable.cpp: green_unswitchable_new uses readonly_state() - test_interpreter_shutdown.py: 7 new subprocess-based tests - CHANGES.rst: release note Made-with: Cursor
Made-with: Cursor
Two fixes to clear_deleteme_list(): 1. Use std::swap instead of vector copy to avoid PythonAllocator (PyMem_Malloc) allocation that crashes during early Py_FinalizeEx on Python < 3.11. 2. Save/restore the pending exception around the cleanup loop so that PyErr_WriteUnraisable/PyErr_Clear inside the loop cannot swallow an unrelated exception (e.g. one set by throw()). Revert mod_getcurrent and PyGreenlet_GetCurrent to use the non-const path so that getcurrent() continues to trigger cross-thread cleanup. Keep green_new/green_unswitchable_new using readonly_state() since construction doesn't need to trigger cleanup. Made-with: Cursor
Greenlet construction (green_new, green_unswitchable_new) must also trigger clear_deleteme_list because existing code and tests depend on it for cross-thread cleanup (e.g. test_dealloc_other_thread uses RawGreenlet() to trigger deleteme processing). With std::swap + exception preservation in clear_deleteme_list, the crash and exception-loss bugs are fixed at the source. The readonly_state / const borrow_current utilities are no longer needed and are removed. Made-with: Cursor
374c98e to
17d17e3
Compare
Three root causes of SIGSEGV during interpreter shutdown on Python < 3.11 were identified through uWSGI worker recycling stress tests and fixed: 1. ThreadState allocated via PyObject_Malloc — pymalloc pools can be disrupted during early Py_FinalizeEx, corrupting the C++ object. Switched to std::malloc/std::free. 2. deleteme vector used PythonAllocator (PyMem_Malloc) — same pool corruption issue. Switched to std::allocator (system malloc). 3. _Py_IsFinalizing() is only set AFTER call_py_exitfuncs and _PyGC_CollectIfEnabled complete, so atexit handlers and __del__ methods could call greenlet.getcurrent() when type objects were already invalidated, crashing in PyType_IsSubtype. An atexit handler registered at module init (LIFO = runs first) now sets a shutdown flag checked by getcurrent(), PyGreenlet_GetCurrent(), and clear_deleteme_list(). All new code is guarded by #if !GREENLET_PY311 — zero impact on Python 3.11+. Verified: 0 segfaults in 200 uWSGI worker recycles on Python 3.9.7 (previously 3-4 crashes in 30 recycles). Made-with: Cursor
17d17e3 to
733a419
Compare
Python 3.13 renamed _Py_IsFinalizing() to Py_IsFinalizing(), requiring #if/#elif/#else chains at every call site. Centralize the version check in greenlet_cpython_compat.hpp so call sites use a single macro and future Python versions need only one update. Made-with: Cursor
The final USS measurement in _check_untracked_memory_thread can pick up tens of KB of OS-level noise (working set trimming, page table updates, thread-local caches) between the loop exit and the assertion. Add a 1 MB tolerance on Windows only, keeping the strict check on Linux/macOS where USS is more stable. Real leaks grow by MBs over 100 iterations, so this tolerance cannot mask genuine issues. Made-with: Cursor
Py_IsFinalizing() has been a public CPython API since Python 3.6 and is available on all Python versions greenlet supports (>=3.9). The private _Py_IsFinalizing() name was removed in Python 3.13, but the public function works on all versions. Drop the custom macro in favor of the standard API — when older Python support is eventually dropped, there is nothing to clean up. Made-with: Cursor
Py_IsFinalizing() only became a public C API in Python 3.13; older versions only expose _Py_IsFinalizing(). Add a two-line macro that maps the public name to the private one on < 3.13, so all call sites use the standard name. Remove the macro when < 3.13 is dropped. Made-with: Cursor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
While regression-testing greenlet 3.2.5 (the backport of PR #495) against Python 3.9.7 under uWSGI, we discovered multiple independent crash paths that were not fixed by PR #495. These crashes affect Python < 3.11.
This PR fixes three root causes of SIGSEGV during
Py_FinalizeExand a latent exception-swallowing bug:clear_deleteme_list()vector allocation crash: The vector copy usedPythonAllocator(PyMem_Malloc), which can SIGSEGV during earlyPy_FinalizeExwhen Python's allocator pools are partially torn down. Replaced withstd::swap(zero-allocation, constant-time) and switcheddeleteme_ttostd::allocator(systemmalloc).ThreadStatememory corruption:ThreadStateobjects were allocated viaPyObject_Malloc, placing them inpymallocpools that can be disrupted during finalization. Switched tostd::malloc/std::freesoThreadStatememory remains valid throughoutPy_FinalizeEx.getcurrent()crash on invalidated type objects:_Py_IsFinalizing()is only set aftercall_py_exitfuncsand_PyGC_CollectIfEnabledcomplete insidePy_FinalizeEx, so code in atexit handlers or__del__methods could still callgreenlet.getcurrent()when type objects had already been invalidated, crashing inPyType_IsSubtype. An atexit handler is now registered at module init (LIFO = runs first) that sets a shutdown flag checked bygetcurrent(),PyGreenlet_GetCurrent(), andclear_deleteme_list().Exception preservation:
clear_deleteme_list()now preserves any pending Python exception around its cleanup loop, fixing a latent bug where an unrelated exception (e.g. one set bythrow()) could be swallowed byPyErr_WriteUnraisable/PyErr_Clear.All new code is guarded by
#if !GREENLET_PY311— zero impact on Python 3.11+.How this was discovered
After PR #495 was merged and 3.2.5 was released, we ran our uWSGI-based crash reproducer (which simulates production worker recycling with
max-requests=1) against Python 3.9.7 with greenlet 3.2.5. The segfaults persisted. Usingfprintfdebug tracing andobjdumpdisassembly of crash backtraces, we narrowed the crashes to three distinct code paths, each requiring its own fix.Root Cause Analysis
Why PR #495's guard doesn't help here
PR #495 added
_Py_IsFinalizing()guards to prevent crashes during greenlet deallocation (GC-triggered, late inPy_FinalizeEx). At that point,_Py_IsFinalizing()is alreadytrue, so the guard works.These new crashes occur earlier — during
call_py_exitfuncs()(atexit handlers) and_PyGC_CollectIfEnabled(), which run before_PyRuntimeState_SetFinalizing():The three crash mechanisms
Allocator disruption: During early
Py_FinalizeEx, Python'spymallocpools can be disrupted. Any C++ object allocated viaPyObject_MallocorPyMem_Malloc(includingThreadStateand itsdeletemevector's internal buffer) may have its memory corrupted. Fix: use systemmallocfor both.Type object invalidation: Python cleanup code (atexit handlers,
__del__methods) can callgreenlet.getcurrent(), which returns anOwnedGreenletsmart pointer. TheGreenletCheckertype validator callsPyType_IsSubtype, which dereferences the greenlet'sob_type— if that type object has been freed or overwritten, SIGSEGV. Fix: an atexit handler registered at module init (last registered = first called in LIFO order) sets a flag before any other cleanup runs.Exception loss:
clear_deleteme_list()runsPy_DECREF+PyErr_WriteUnraisable+PyErr_Clearin a loop, which can clear a pending exception set by an earlierthrow()call. Fix: save and restore the exception state around the loop usingPyErrPieces.Test plan
test_interpreter_shutdown.pypasses (subprocess-based shutdown tests)test_throw_exception_not_lostpasses (exception preservation)test_dealloc_other_threadpasses (cross-thread cleanup)test_issue251_killing_cross_thread_leaks_listpassesBackport note
If the maintainer plans a 3.2.x backport (for Python 3.9 support), we recommend a full sync of master into the 3.2.x branch rather than a surgical cherry-pick. All C++ changes between 3.2.4 and 3.3.x are behind
#ifdef Py_GIL_DISABLEDor#if GREENLET_PY312+guards — they compile to nothing on Python 3.9, so there is zero runtime risk. Keeping the branches nearly identical (differing only inpyproject.tomland CI config) makes future maintenance significantly easier.I can create that follow up PR once this one has been reviewed.