Skip to content

Commit 733a419

Browse files
committed
Fix Python < 3.11 crashes during Py_FinalizeEx (uWSGI worker recycling)
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
1 parent ba0301d commit 733a419

5 files changed

Lines changed: 146 additions & 18 deletions

File tree

CHANGES.rst

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,38 @@
55
3.3.3 (unreleased)
66
==================
77

8-
- Fix a second crash path during interpreter shutdown (observed on
9-
Python < 3.11): ``clear_deleteme_list()`` could crash when its
10-
``std::vector`` copy allocated through ``PythonAllocator``
11-
(``PyMem_Malloc``) during early ``Py_FinalizeEx`` cleanup (before
12-
``_Py_IsFinalizing()`` is set). The vector contents are now moved
13-
with ``std::swap`` (zero-allocation, constant-time) instead of
14-
copied. Additionally, ``clear_deleteme_list()`` now preserves any
15-
pending Python exception around its cleanup loop, fixing a latent
16-
bug where an unrelated exception (e.g. one set by ``throw()``) could
17-
be swallowed by ``PyErr_WriteUnraisable`` / ``PyErr_Clear`` inside
18-
the loop. This is distinct from the dealloc crash fixed in 3.3.2
8+
- Fix multiple crash paths during interpreter shutdown on Python < 3.11
9+
(observed with uWSGI worker recycling). Three root causes were
10+
identified and fixed:
11+
12+
1. ``clear_deleteme_list()`` used a ``PythonAllocator``-backed vector
13+
copy (``PyMem_Malloc``), which could SIGSEGV during early
14+
``Py_FinalizeEx`` when Python's allocator pools are partially torn
15+
down. Replaced with ``std::swap`` (zero-allocation,
16+
constant-time) and switched the ``deleteme`` vector to
17+
``std::allocator`` (system ``malloc``).
18+
19+
2. ``ThreadState`` objects were allocated via ``PyObject_Malloc``,
20+
placing them in ``pymalloc`` pools that can be disrupted during
21+
finalization. Switched to ``std::malloc`` / ``std::free`` so
22+
``ThreadState`` memory remains valid throughout ``Py_FinalizeEx``.
23+
24+
3. ``_Py_IsFinalizing()`` is only set *after* ``call_py_exitfuncs``
25+
and ``_PyGC_CollectIfEnabled`` complete inside ``Py_FinalizeEx``,
26+
so code in atexit handlers or ``__del__`` methods could still call
27+
``greenlet.getcurrent()`` when type objects had already been
28+
invalidated, crashing in ``PyType_IsSubtype``. An atexit handler
29+
is now registered at module init (LIFO = runs first) that sets a
30+
shutdown flag checked by ``getcurrent()``,
31+
``PyGreenlet_GetCurrent()``, and ``clear_deleteme_list()``.
32+
33+
Additionally, ``clear_deleteme_list()`` now preserves any pending
34+
Python exception around its cleanup loop, fixing a latent bug where
35+
an unrelated exception (e.g. one set by ``throw()``) could be
36+
swallowed by ``PyErr_WriteUnraisable`` / ``PyErr_Clear`` inside the
37+
loop.
38+
39+
This is distinct from the dealloc crash fixed in 3.3.2
1940
(`PR #495
2041
<https://github.com/python-greenlet/greenlet/pull/495>`_). See `PR #499
2142
<https://github.com/python-greenlet/greenlet/pull/499>`_ by Nicolas

src/greenlet/CObjects.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ extern "C" {
2929
static PyGreenlet*
3030
PyGreenlet_GetCurrent(void)
3131
{
32+
#if !GREENLET_PY311
33+
if (g_greenlet_shutting_down || _Py_IsFinalizing()) {
34+
return nullptr;
35+
}
36+
#endif
3237
return GET_THREAD_STATE().state().get_current().relinquish_ownership();
3338
}
3439

src/greenlet/PyModule.cpp

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,30 @@ using greenlet::ThreadState;
1717
# pragma clang diagnostic ignored "-Wunused-variable"
1818
#endif
1919

20+
// On Python < 3.11, _Py_IsFinalizing() is only set AFTER
21+
// call_py_exitfuncs and _PyGC_CollectIfEnabled finish inside
22+
// Py_FinalizeEx. Code running in atexit handlers or __del__
23+
// methods can still call greenlet.getcurrent(), but by that
24+
// time type objects may have been invalidated, causing
25+
// SIGSEGV in PyType_IsSubtype. This flag is set by an atexit
26+
// handler registered at module init (LIFO = runs first).
27+
#if !GREENLET_PY311
28+
int g_greenlet_shutting_down = 0;
29+
30+
static PyObject*
31+
_greenlet_atexit_callback(PyObject* UNUSED(self), PyObject* UNUSED(args))
32+
{
33+
g_greenlet_shutting_down = 1;
34+
Py_RETURN_NONE;
35+
}
36+
37+
static PyMethodDef _greenlet_atexit_method = {
38+
"_greenlet_cleanup", _greenlet_atexit_callback,
39+
METH_NOARGS, NULL
40+
};
41+
#endif
42+
43+
2044
PyDoc_STRVAR(mod_getcurrent_doc,
2145
"getcurrent() -> greenlet\n"
2246
"\n"
@@ -26,6 +50,11 @@ PyDoc_STRVAR(mod_getcurrent_doc,
2650
static PyObject*
2751
mod_getcurrent(PyObject* UNUSED(module))
2852
{
53+
#if !GREENLET_PY311
54+
if (g_greenlet_shutting_down || _Py_IsFinalizing()) {
55+
Py_RETURN_NONE;
56+
}
57+
#endif
2958
return GET_THREAD_STATE().state().get_current().relinquish_ownership_o();
3059
}
3160

src/greenlet/TThreadState.hpp

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#ifndef GREENLET_THREAD_STATE_HPP
22
#define GREENLET_THREAD_STATE_HPP
33

4+
#include <cstdlib>
45
#include <ctime>
56
#include <stdexcept>
67
#include <atomic>
@@ -23,6 +24,13 @@ using greenlet::refs::CreatedModule;
2324
using greenlet::refs::PyErrPieces;
2425
using greenlet::refs::NewReference;
2526

27+
// Defined in PyModule.cpp; set by an atexit handler to signal
28+
// that the interpreter is shutting down. Only needed on
29+
// Python < 3.11 where _Py_IsFinalizing() is set too late.
30+
#if !GREENLET_PY311
31+
extern int g_greenlet_shutting_down;
32+
#endif
33+
2634
namespace greenlet {
2735
/**
2836
* Thread-local state of greenlets.
@@ -100,7 +108,13 @@ class ThreadState {
100108
/* Strong reference to the trace function, if any. */
101109
OwnedObject tracefunc;
102110

103-
typedef std::vector<PyGreenlet*, PythonAllocator<PyGreenlet*> > deleteme_t;
111+
// Use std::allocator (malloc/free) instead of PythonAllocator
112+
// (PyMem_Malloc) for the deleteme list. During Py_FinalizeEx on
113+
// Python < 3.11, the PyObject_Malloc pool that holds ThreadState
114+
// can be disrupted, corrupting any PythonAllocator-backed
115+
// containers. Using std::allocator makes this vector independent
116+
// of Python's allocator lifecycle.
117+
typedef std::vector<PyGreenlet*> deleteme_t;
104118
/* A vector of raw PyGreenlet pointers representing things that need
105119
deleted when this thread is running. The vector owns the
106120
references, but you need to manually INCREF/DECREF as you use
@@ -120,7 +134,6 @@ class ThreadState {
120134
static std::clock_t _clocks_used_doing_gc;
121135
#endif
122136
static ImmortalString get_referrers_name;
123-
static PythonAllocator<ThreadState> allocator;
124137

125138
G_NO_COPIES_OF_CLS(ThreadState);
126139

@@ -146,15 +159,21 @@ class ThreadState {
146159

147160

148161
public:
149-
static void* operator new(size_t UNUSED(count))
162+
// Allocate ThreadState with malloc/free rather than Python's object
163+
// allocator. ThreadState outlives many Python objects and must
164+
// remain valid throughout Py_FinalizeEx. On Python < 3.11,
165+
// PyObject_Malloc pools can be disrupted during early finalization,
166+
// corrupting any C++ objects stored in them.
167+
static void* operator new(size_t count)
150168
{
151-
return ThreadState::allocator.allocate(1);
169+
void* p = std::malloc(count);
170+
if (!p) throw std::bad_alloc();
171+
return p;
152172
}
153173

154174
static void operator delete(void* ptr)
155175
{
156-
return ThreadState::allocator.deallocate(static_cast<ThreadState*>(ptr),
157-
1);
176+
std::free(ptr);
158177
}
159178

160179
static void init()
@@ -292,6 +311,26 @@ class ThreadState {
292311
deleteme_t copy;
293312
std::swap(copy, this->deleteme);
294313

314+
// During Py_FinalizeEx cleanup, the GC or atexit handlers
315+
// may have already collected objects in this list, leaving
316+
// dangling pointers. Attempting Py_DECREF on freed memory
317+
// causes a SIGSEGV. On Python < 3.11,
318+
// g_greenlet_shutting_down covers the early stages
319+
// (before _Py_IsFinalizing() is set).
320+
#if !GREENLET_PY311
321+
if (g_greenlet_shutting_down || _Py_IsFinalizing()) {
322+
return;
323+
}
324+
#elif GREENLET_PY313
325+
if (Py_IsFinalizing()) {
326+
return;
327+
}
328+
#else
329+
if (_Py_IsFinalizing()) {
330+
return;
331+
}
332+
#endif
333+
295334
// Preserve any pending exception so that cleanup-triggered
296335
// errors don't accidentally swallow an unrelated exception
297336
// (e.g. one set by throw() before a switch).
@@ -538,7 +577,6 @@ class ThreadState {
538577
};
539578

540579
ImmortalString ThreadState::get_referrers_name(nullptr);
541-
PythonAllocator<ThreadState> ThreadState::allocator;
542580
#ifdef Py_GIL_DISABLED
543581
std::atomic<std::clock_t> ThreadState::_clocks_used_doing_gc(0);
544582
#else

src/greenlet/greenlet.cpp

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,41 @@ greenlet_internal_mod_init() noexcept
294294
#ifdef Py_GIL_DISABLED
295295
PyUnstable_Module_SetGIL(m.borrow(), Py_MOD_GIL_NOT_USED);
296296
#endif
297+
298+
#if !GREENLET_PY311
299+
// Register an atexit handler that sets g_greenlet_shutting_down.
300+
// Python's atexit is LIFO: registered last = called first. By
301+
// registering here (at import time, after most other libraries),
302+
// our handler runs before their cleanup code, which may try to
303+
// call greenlet.getcurrent() on objects whose type has been
304+
// invalidated. _Py_IsFinalizing() alone is insufficient
305+
// because it is only set AFTER call_py_exitfuncs completes.
306+
{
307+
PyObject* atexit_mod = PyImport_ImportModule("atexit");
308+
if (atexit_mod) {
309+
PyObject* register_fn = PyObject_GetAttrString(atexit_mod, "register");
310+
if (register_fn) {
311+
extern PyMethodDef _greenlet_atexit_method;
312+
PyObject* callback = PyCFunction_New(&_greenlet_atexit_method, NULL);
313+
if (callback) {
314+
PyObject* args = PyTuple_Pack(1, callback);
315+
if (args) {
316+
PyObject* result = PyObject_Call(register_fn, args, NULL);
317+
Py_XDECREF(result);
318+
Py_DECREF(args);
319+
}
320+
Py_DECREF(callback);
321+
}
322+
Py_DECREF(register_fn);
323+
}
324+
Py_DECREF(atexit_mod);
325+
}
326+
// Non-fatal: if atexit registration fails, we still have
327+
// the _Py_IsFinalizing() fallback.
328+
PyErr_Clear();
329+
}
330+
#endif
331+
297332
return m.borrow(); // But really it's the main reference.
298333
}
299334
catch (const LockInitError& e) {

0 commit comments

Comments
 (0)