Skip to content

Commit 004e1e9

Browse files
committed
Remove the atexit callback.
This callback caused greenlet APIs to become unavailable far too soon during interpreter shutdown. Now they remain available while all ``atexit`` callbacks run; using greenlet APIs from atexit callbacks registered in any order is a valid thing to do and happens naturally with gevent monkey-patching. A careful, thorough reading of the CPython documentation and the CPython source code for all supported versions of Python (3.10+) indicates that any gating needing to be done is correctly handled with ``Py_IsFinalizing``. The comments in PR python-greenlet#499 that added the callback were wrong: it's not possible to access partially torn-down state during ``atexit``. And the tests provided in that PR did not demonstrate any crashes (they pass with or without the ``atexit`` callback). CPython shuts things down in the following order: 1. Attempt to wait for all non-daemon threads to finish. 2. Invoke any pending calls. 3. Invoke ``atexit`` callbacks. At this point, it is guaranteed that the interpreter is still fully operational, the import machinery still works, etc. All such callbacks can successfully use greenlet APIs. 4. Finalize any sub-intepreters in newer versions. greenlet doesn't support sub-interpreters, so this is inconsequential. 5. Detach any remaining threads in newer versions. 6. Set ``Py_IsFinalizing`` to true. Any other threads still remaining will no longer be able to run Python code. All cleanup operations continue in this thread. At this point, ``getcurrent`` will start returning ``None`` or raising an exception (C API). 7. Garbage collect threads (active objects on the call stack). 8. Run cyclic garbage collection. 9. Only now does the interpreter begin to tear down module state, beginning by clearing out module dictionaries and allowing finalizers/weakref to be cleared. Up to and through the beginning of this process, greenlet APIs are safe to call, and greenlet objects can be used (switched/thrown). At some point, this may become untrue, but all unreachable greenlet objects (which should be anything not stashed away in a C extension). greenlet can't do anything about extant objects that may still have methods called on them, but it can prevent getting access to implicit objects that may be getting torn down: that's why getcurrent behaves the way it does (any exceptions generated during at least steps 7, 8, 9 are "unraisable" and just get printed). Note that the CPython documentation specifically calls out the fact that modules may be finalized in any order, so modules that rely on other modules MUST be coded defensively. The long and short is that greenlet can't do anything reasonable to protect other modules from accessing state that may be torn down (``atexit`` is too soon; a PyCapsule destructor may be too late or never get fired; module ``m_clear`` and ``m_free`` functions may never get called). It's up to other C modules to check for interpreter finalization and be aware that any other C modules they use may no longer be valid at that point.
1 parent b784a69 commit 004e1e9

8 files changed

Lines changed: 226 additions & 180 deletions

File tree

CHANGES.rst

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,24 @@
22
Changes
33
=========
44

5-
3.4.1 (unreleased)
5+
3.5.0 (unreleased)
66
==================
77

8-
- Nothing changed yet.
8+
- Remove the ``atexit`` callback. This callback caused greenlet APIs
9+
to become unavailable far too soon during interpreter shutdown. Now
10+
they remain available while all ``atexit`` callbacks run. Sometime
11+
after ``Py_IsFinalizing`` becomes true, they may begin misbehaving.
12+
Because the order in which C extensions are finalized is undefined,
13+
C extensions that are sensitive to this need to check the results of
14+
that function before invoking greenlet APIs. As a convenience,
15+
``PyGreenlet_GetCurrent`` sets an exception and returns ``NULL``
16+
when this happens (and ``greenlet.getcurrent`` begins returning
17+
``None``); other greenlet C API functions have undefined behaviour.
18+
Methods invoked directly on pre-existing ``greenlet.greenlet``
19+
objects will continue to function at least until the greenlet C
20+
extension has been garbage collected and finalized.
21+
22+
See `issue 507 <https://github.com/python-greenlet/greenlet/issues/507>`_.
923

1024

1125
3.4.0 (2026-04-08)

docs/c_api.rst

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,14 @@ Exceptions
3030
Functions
3131
=========
3232

33+
.. important::
34+
35+
Because the order in which extension modules are destroyed when the
36+
Python interpreter is finalized is undefined, it is undefined
37+
behaviour to call these APIs when ``Py_IsFinalizing`` returns true,
38+
unless otherwise documented. This is because the internal state of
39+
the greenlet module may have been torn down already.
40+
3341
.. c:function:: void PyGreenlet_Import(void)
3442
3543
A macro that imports the greenlet module and initializes the C API. This
@@ -67,6 +75,20 @@ Functions
6775
6876
Returns the currently active greenlet object.
6977
78+
If called during interpreter finalization, returns ``NULL``
79+
and raises a :exc:`RuntimeError`.
80+
81+
.. versionchanged:: 3.4.0
82+
Began returning ``NULL`` during interpreter shutdown.
83+
This implementation returned ``NULL`` too early, while the
84+
interpreter state was still guaranteed to be valid (during
85+
``atexit`` handlers). This has been corrected in 3.5.
86+
.. versionchanged:: 3.5.0
87+
Now sets an exception before returning ``NULL``. This prevents
88+
a :exc:`SystemError` from being generated if this API was
89+
exposed directly to Python, and prevents a crash if this API
90+
was being called by Cython-generated code.
91+
7092
7193
.. c:function:: PyGreenlet* PyGreenlet_New(PyObject* run, PyObject* parent)
7294

src/greenlet/CObjects.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ static PyGreenlet*
3030
PyGreenlet_GetCurrent(void)
3131
{
3232
if (greenlet::IsShuttingDown()) {
33+
PyErr_SetString(PyExc_RuntimeError, "greenlet is being finalized");
3334
return nullptr;
3435
}
3536
return GET_THREAD_STATE().state().get_current().relinquish_ownership();

src/greenlet/PyModule.cpp

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,6 @@ using greenlet::ThreadState;
1818
#endif
1919

2020

21-
static PyObject*
22-
_greenlet_atexit_callback(PyObject* UNUSED(self), PyObject* UNUSED(args))
23-
{
24-
greenlet::g_greenlet_shutting_down = 1;
25-
Py_RETURN_NONE;
26-
}
27-
28-
static PyMethodDef _greenlet_atexit_method = {
29-
"_greenlet_cleanup", _greenlet_atexit_callback,
30-
METH_NOARGS, NULL
31-
};
32-
33-
3421
PyDoc_STRVAR(mod_getcurrent_doc,
3522
"getcurrent() -> greenlet\n"
3623
"\n"

src/greenlet/greenlet.cpp

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -295,25 +295,6 @@ greenlet_internal_mod_init() noexcept
295295
PyUnstable_Module_SetGIL(m.borrow(), Py_MOD_GIL_NOT_USED);
296296
#endif
297297

298-
// Register an atexit handler that sets
299-
// g_greenlet_shutting_down. Python's atexit is LIFO:
300-
// registered last = called first. By registering here (at
301-
// import time, after most other libraries), our handler runs
302-
// before their cleanup code, which may try to call
303-
// greenlet.getcurrent() on objects whose type has been
304-
// invalidated. _Py_IsFinalizing() alone is insufficient on
305-
// ALL Python versions because it is only set AFTER atexit
306-
// handlers complete inside Py_FinalizeEx.
307-
{
308-
NewReference atexit_mod(Require(PyImport_ImportModule("atexit")));
309-
OwnedObject register_fn = atexit_mod.PyRequireAttr("register");
310-
NewReference callback(Require(
311-
PyCFunction_New(&_greenlet_atexit_method, NULL)));
312-
NewReference args(Require(PyTuple_Pack(1, callback.borrow())));
313-
NewReference result(Require(
314-
PyObject_Call(register_fn.borrow(), args.borrow(), NULL)));
315-
}
316-
317298
return m.borrow(); // But really it's the main reference.
318299
}
319300
catch (const LockInitError& e) {

src/greenlet/greenlet_refs.hpp

Lines changed: 42 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -27,25 +27,52 @@ using std::endl;
2727
namespace greenlet
2828
{
2929
class Greenlet;
30-
// _Py_IsFinalizing() is only set AFTER atexit handlers complete
31-
// inside Py_FinalizeEx on ALL Python versions (including 3.11+).
32-
// Code running in atexit handlers (e.g. uWSGI plugin cleanup
33-
// calling Py_FinalizeEx, New Relic agent shutdown) can still call
34-
// greenlet.getcurrent(), but by that time type objects or
35-
// internal state may have been invalidated. This flag is set by
36-
// an atexit handler registered at module init (LIFO = runs
37-
// first).
38-
//
39-
// Because this is only set from an atexit handler, by which point
40-
// we're single threaded, there should be no need to make it
41-
// std::atomic<int>.
42-
// TODO: Move this to the GreenletGlobals object?
43-
static int g_greenlet_shutting_down;
4430

4531
static inline bool
4632
IsShuttingDown()
4733
{
48-
return greenlet::g_greenlet_shutting_down || Py_IsFinalizing();
34+
// This used to check a flag set by an ``atexit`` callback.
35+
// This was wrong: the interpreter is still fully functional
36+
// while *all* atexit callbacks are run, and it is perfectly
37+
// valid for an atexit callback that runs after our atexit
38+
// callback (i.e., registered first/before ours) to want to
39+
// make use of greenlet services --- this comes up easily with
40+
// gevent monkey-patching. Almost immediately after atexit callbacks,
41+
// and before any destructive action is taken, Python arranges
42+
// for Py_IsFinalizing to become true.
43+
44+
// It may see me could potentially tighten this check even more (and
45+
// eliminate a function call) by setting a flag in a
46+
// destructor function for our PyCapsule object (_C_API) to
47+
// determine when we're shutting down. ``Py_IsFinalizing``
48+
// becomes true relatively early in the shutdown process,
49+
// while Capsule destructor functions only run when the module
50+
// has actually been torn down --- well, when all of its dicts are
51+
// cleared and collected; recall that because we use
52+
// single-phase init, there is a "hidden" copy of the module
53+
// dict kept by CPython internals used to re-populate a module
54+
// if greenlet is imported twice, so Python code can't trigger
55+
// C_API to get GC'd early without seriously poking at CPython
56+
// internals, e.g., by using `gc.get_referrers` to find the
57+
// hidden dict. However, C extensions could have INCREF the
58+
// capsule object and prevent it from *ever* getting torn
59+
// down, so this isn't reliable.
60+
61+
// We could probably be even "smarter" and replace values in
62+
// _PyGreenlet_API with different values at destruction time.
63+
// For the PyObject* returning APIs, we could replace them
64+
// with versions that set an exception and return null --- the
65+
// benefit being that we don't have to include a
66+
// Py_IsFinalizing() call in the normal path; int returning
67+
// APIs would be handled on a case-by-case basis; unclear what
68+
// to do with the types. This is of questionable benefit
69+
// though because by the time our destructor is called, our
70+
// module is about to be destroyed which may take our
71+
// allocated storage with it (if CPython ever dynamically
72+
// unloads loaded shared libraries, which as of 3.14 it never
73+
// does).
74+
75+
return Py_IsFinalizing();
4976
}
5077

5178
namespace refs

src/greenlet/tests/_test_extension.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,13 @@ test_throw_exact(PyObject* UNUSED(self), PyObject* args)
189189
Py_RETURN_NONE;
190190
}
191191

192+
static PyObject*
193+
getcurrent_api(PyObject* UNUSED(self))
194+
{
195+
return (PyObject*)PyGreenlet_GetCurrent();
196+
197+
}
198+
192199
static PyMethodDef test_methods[] = {
193200
{"test_switch",
194201
(PyCFunction)test_switch,
@@ -227,6 +234,11 @@ static PyMethodDef test_methods[] = {
227234
(PyCFunction)test_throw_exact,
228235
METH_VARARGS,
229236
"Throw exactly the arguments given at the provided greenlet"},
237+
{
238+
"getcurrent_api",
239+
(PyCFunction)getcurrent_api,
240+
METH_NOARGS,
241+
"Direct call to the PyGreenlet_GetCurrent API."},
230242
{NULL, NULL, 0, NULL}
231243
};
232244

0 commit comments

Comments
 (0)