Merge pull request #508 from python-greenlet/issue507-remove-atexit

Remove the atexit callback.

Author: jamadden

CHANGES.rst

@@ -2,10 +2,24 @@
  Changes
 =========
 
-3.4.1 (unreleased)
+3.5.0 (unreleased)
 ==================
 
-- Nothing changed yet.
+- 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. Sometime
+  after ``Py_IsFinalizing`` becomes true, they may begin misbehaving.
+  Because the order in which C extensions are finalized is undefined,
+  C extensions that are sensitive to this need to check the results of
+  that function before invoking greenlet APIs. As a convenience,
+  ``PyGreenlet_GetCurrent`` sets an exception and returns ``NULL``
+  when this happens (and ``greenlet.getcurrent`` begins returning
+  ``None``); other greenlet C API functions have undefined behaviour.
+  Methods invoked directly on pre-existing ``greenlet.greenlet``
+  objects will continue to function at least until the greenlet C
+  extension has been garbage collected and finalized.
+
+  See `issue 507 <https://github.com/python-greenlet/greenlet/issues/507>`_.
 
 
 3.4.0 (2026-04-08)

docs/c_api.rst

@@ -30,6 +30,14 @@ Exceptions
 Functions
 =========
 
+.. important::
+
+   Because the order in which extension modules are destroyed when the
+   Python interpreter is finalized is undefined, it is undefined
+   behaviour to call these APIs when ``Py_IsFinalizing`` returns true,
+   unless otherwise documented. This is because the internal state of
+   the greenlet module may have been torn down already.
+
 .. c:function:: void PyGreenlet_Import(void)
 
    A macro that imports the greenlet module and initializes the C API. This
@@ -67,6 +75,20 @@ Functions
 
     Returns the currently active greenlet object.
 
+    If called during interpreter finalization, returns ``NULL``
+    and raises a :exc:`RuntimeError`.
+
+    .. versionchanged:: 3.4.0
+       Began returning ``NULL`` during interpreter shutdown.
+       This implementation returned ``NULL`` too early, while the
+       interpreter state was still guaranteed to be valid (during
+       ``atexit`` handlers). This has been corrected in 3.5.
+    .. versionchanged:: 3.5.0
+       Now sets an exception before returning ``NULL``. This prevents
+       a :exc:`SystemError` from being generated if this API was
+       exposed directly to Python, and prevents a crash if this API
+       was being called by Cython-generated code.
+
 
 .. c:function:: PyGreenlet* PyGreenlet_New(PyObject* run, PyObject* parent)
 

src/greenlet/CObjects.cpp

@@ -30,6 +30,7 @@ static PyGreenlet*
 PyGreenlet_GetCurrent(void)
 {
     if (greenlet::IsShuttingDown()) {
+        PyErr_SetString(PyExc_RuntimeError, "greenlet is being finalized");
         return nullptr;
     }
     return GET_THREAD_STATE().state().get_current().relinquish_ownership();

src/greenlet/PyModule.cpp

@@ -18,19 +18,6 @@ using greenlet::ThreadState;
 #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"

src/greenlet/greenlet.cpp

@@ -295,25 +295,6 @@ greenlet_internal_mod_init() noexcept
         PyUnstable_Module_SetGIL(m.borrow(), Py_MOD_GIL_NOT_USED);
 #endif
 
-        // 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.
-        {
-            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)));
-        }
-
         return m.borrow(); // But really it's the main reference.
     }
     catch (const LockInitError& e) {

src/greenlet/greenlet_refs.hpp

@@ -27,25 +27,52 @@ 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<int>.
-    // TODO: Move this to the GreenletGlobals object?
-    static int g_greenlet_shutting_down;
 
     static inline bool
     IsShuttingDown()
     {
-        return greenlet::g_greenlet_shutting_down || Py_IsFinalizing();
+        // This used to check a flag set by an ``atexit`` callback.
+        // This was wrong: the interpreter is still fully functional
+        // while *all* atexit callbacks are run, and it is perfectly
+        // valid for an atexit callback that runs after our atexit
+        // callback (i.e., registered first/before ours) to want to
+        // make use of greenlet services --- this comes up easily with
+        // gevent monkey-patching. Almost immediately after atexit callbacks,
+        // and before any destructive action is taken, Python arranges
+        // for Py_IsFinalizing to become true.
+
+        // It may see me could potentially tighten this check even more (and
+        // eliminate a function call) by setting a flag in a
+        // destructor function for our PyCapsule object (_C_API) to
+        // determine when we're shutting down. ``Py_IsFinalizing``
+        // becomes true relatively early in the shutdown process,
+        // while Capsule destructor functions only run when the module
+        // has actually been torn down --- well, when all of its dicts are
+        // cleared and collected; recall that because we use
+        // single-phase init, there is a "hidden" copy of the module
+        // dict kept by CPython internals used to re-populate a module
+        // if greenlet is imported twice, so Python code can't trigger
+        // C_API to get GC'd early without seriously poking at CPython
+        // internals, e.g., by using `gc.get_referrers` to find the
+        // hidden dict. However, C extensions could have INCREF the
+        // capsule object and prevent it from *ever* getting torn
+        // down, so this isn't reliable.
+
+        // We could probably be even "smarter" and replace values in
+        // _PyGreenlet_API with different values at destruction time.
+        // For the PyObject* returning APIs, we could replace them
+        // with versions that set an exception and return null --- the
+        // benefit being that we don't have to include a
+        // Py_IsFinalizing() call in the normal path; int returning
+        // APIs would be handled on a case-by-case basis; unclear what
+        // to do with the types. This is of questionable benefit
+        // though because by the time our destructor is called, our
+        // module is about to be destroyed which may take our
+        // allocated storage with it (if CPython ever dynamically
+        // unloads loaded shared libraries, which as of 3.14 it never
+        // does).
+
+        return Py_IsFinalizing();
     }
 
     namespace refs

src/greenlet/tests/_test_extension.c

@@ -189,6 +189,13 @@ test_throw_exact(PyObject* UNUSED(self), PyObject* args)
     Py_RETURN_NONE;
 }
 
+static PyObject*
+getcurrent_api(PyObject* UNUSED(self))
+{
+    return (PyObject*)PyGreenlet_GetCurrent();
+
+}
+
 static PyMethodDef test_methods[] = {
     {"test_switch",
      (PyCFunction)test_switch,
@@ -227,6 +234,11 @@ static PyMethodDef test_methods[] = {
      (PyCFunction)test_throw_exact,
      METH_VARARGS,
      "Throw exactly the arguments given at the provided greenlet"},
+    {
+        "getcurrent_api",
+        (PyCFunction)getcurrent_api,
+        METH_NOARGS,
+        "Direct call to the PyGreenlet_GetCurrent API."},
     {NULL, NULL, 0, NULL}
 };