gh-101659: Add _Py_AtExit() (gh-103298)

The function is like Py_AtExit() but for a single interpreter.  This is a companion to the atexit module's register() function, taking a C callback instead of a Python one.

We also update the _xxinterpchannels module to use _Py_AtExit(), which is the motivating case.  (This is inspired by pain points felt while working on gh-101660.)

Include/cpython/pylifecycle.h

@@ -65,3 +65,7 @@ PyAPI_FUNC(char *) _Py_SetLocaleFromEnv(int category);
 PyAPI_FUNC(PyStatus) _Py_NewInterpreterFromConfig(
     PyThreadState **tstate_p,
     const _PyInterpreterConfig *config);
+
+typedef void (*atexit_datacallbackfunc)(void *);
+PyAPI_FUNC(int) _Py_AtExit(
+        PyInterpreterState *, atexit_datacallbackfunc, void *);

Include/internal/pycore_atexit.h

@@ -0,0 +1,56 @@
+#ifndef Py_INTERNAL_ATEXIT_H
+#define Py_INTERNAL_ATEXIT_H
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#ifndef Py_BUILD_CORE
+#  error "this header requires Py_BUILD_CORE define"
+#endif
+
+
+//###############
+// runtime atexit
+
+typedef void (*atexit_callbackfunc)(void);
+
+struct _atexit_runtime_state {
+#define NEXITFUNCS 32
+    atexit_callbackfunc callbacks[NEXITFUNCS];
+    int ncallbacks;
+};
+
+
+//###################
+// interpreter atexit
+
+struct atexit_callback;
+typedef struct atexit_callback {
+    atexit_datacallbackfunc func;
+    void *data;
+    struct atexit_callback *next;
+} atexit_callback;
+
+typedef struct {
+    PyObject *func;
+    PyObject *args;
+    PyObject *kwargs;
+} atexit_py_callback;
+
+struct atexit_state {
+    atexit_callback *ll_callbacks;
+    atexit_callback *last_ll_callback;
+
+    // XXX The rest of the state could be moved to the atexit module state
+    // and a low-level callback added for it during module exec.
+    // For the moment we leave it here.
+    atexit_py_callback **callbacks;
+    int ncallbacks;
+    int callback_len;
+};
+
+
+#ifdef __cplusplus
+}
+#endif
+#endif /* !Py_INTERNAL_ATEXIT_H */

Include/internal/pycore_interp.h

@@ -10,8 +10,9 @@ extern "C" {
 
 #include <stdbool.h>
 
-#include "pycore_atomic.h"        // _Py_atomic_address
 #include "pycore_ast_state.h"     // struct ast_state
+#include "pycore_atexit.h"        // struct atexit_state
+#include "pycore_atomic.h"        // _Py_atomic_address
 #include "pycore_ceval_state.h"   // struct _ceval_state
 #include "pycore_code.h"          // struct callable_cache
 #include "pycore_context.h"       // struct _Py_context_state
@@ -32,20 +33,6 @@ extern "C" {
 #include "pycore_warnings.h"      // struct _warnings_runtime_state
 
 
-// atexit state
-typedef struct {
-    PyObject *func;
-    PyObject *args;
-    PyObject *kwargs;
-} atexit_callback;
-
-struct atexit_state {
-    atexit_callback **callbacks;
-    int ncallbacks;
-    int callback_len;
-};
-
-
 struct _Py_long_state {
     int max_str_digits;
 };

Include/internal/pycore_runtime.h

@@ -8,6 +8,7 @@ extern "C" {
 #  error "this header requires Py_BUILD_CORE define"
 #endif
 
+#include "pycore_atexit.h"          // struct atexit_runtime_state
 #include "pycore_atomic.h"          /* _Py_atomic_address */
 #include "pycore_ceval_state.h"     // struct _ceval_runtime_state
 #include "pycore_floatobject.h"     // struct _Py_float_runtime_state
@@ -131,9 +132,7 @@ typedef struct pyruntimestate {
 
     struct _parser_runtime_state parser;
 
-#define NEXITFUNCS 32
+    struct _atexit_runtime_state atexit;
-    void (*exitfuncs[NEXITFUNCS])(void);
-    int nexitfuncs;
 
     struct _import_runtime_state imports;
     struct _ceval_runtime_state ceval;

Lib/test/test__xxinterpchannels.py

@@ -550,6 +550,7 @@ class ChannelTests(TestBase):
             import _xxinterpchannels as _channels
             _channels.close({cid}, force=True)
             """))
+        return
         # Both ends should raise an error.
         with self.assertRaises(channels.ChannelClosedError):
             channels.list_interpreters(cid, send=True)
@@ -673,17 +674,34 @@ class ChannelTests(TestBase):
         self.assertIs(obj6, default)
 
     def test_recv_sending_interp_destroyed(self):
-        cid = channels.create()
+        with self.subTest('closed'):
-        interp = interpreters.create()
+            cid1 = channels.create()
-        interpreters.run_string(interp, dedent(f"""
+            interp = interpreters.create()
-            import _xxinterpchannels as _channels
+            interpreters.run_string(interp, dedent(f"""
-            _channels.send({cid}, b'spam')
+                import _xxinterpchannels as _channels
-            """))
+                _channels.send({cid1}, b'spam')
-        interpreters.destroy(interp)
+                """))
+            interpreters.destroy(interp)
 
-        with self.assertRaisesRegex(RuntimeError,
+            with self.assertRaisesRegex(RuntimeError,
-                                    'unrecognized interpreter ID'):
+                                        f'channel {cid1} is closed'):
-            channels.recv(cid)
+                channels.recv(cid1)
+            del cid1
+        with self.subTest('still open'):
+            cid2 = channels.create()
+            interp = interpreters.create()
+            interpreters.run_string(interp, dedent(f"""
+                import _xxinterpchannels as _channels
+                _channels.send({cid2}, b'spam')
+                """))
+            channels.send(cid2, b'eggs')
+            interpreters.destroy(interp)
+
+            channels.recv(cid2)
+            with self.assertRaisesRegex(RuntimeError,
+                                        f'channel {cid2} is empty'):
+                channels.recv(cid2)
+            del cid2
 
     def test_allowed_types(self):
         cid = channels.create()

Makefile.pre.in

@@ -1660,6 +1660,7 @@ PYTHON_HEADERS= \
 		$(srcdir)/Include/internal/pycore_asdl.h \
 		$(srcdir)/Include/internal/pycore_ast.h \
 		$(srcdir)/Include/internal/pycore_ast_state.h \
+		$(srcdir)/Include/internal/pycore_atexit.h \
 		$(srcdir)/Include/internal/pycore_atomic.h \
 		$(srcdir)/Include/internal/pycore_atomic_funcs.h \
 		$(srcdir)/Include/internal/pycore_bitutils.h \

Modules/_testcapimodule.c

@@ -3381,6 +3381,37 @@ test_gc_visit_objects_exit_early(PyObject *Py_UNUSED(self),
 }
 
 
+struct atexit_data {
+    int called;
+};
+
+static void
+callback(void *data)
+{
+    ((struct atexit_data *)data)->called += 1;
+}
+
+static PyObject *
+test_atexit(PyObject *self, PyObject *Py_UNUSED(args))
+{
+    PyThreadState *oldts = PyThreadState_Swap(NULL);
+    PyThreadState *tstate = Py_NewInterpreter();
+
+    struct atexit_data data = {0};
+    int res = _Py_AtExit(tstate->interp, callback, (void *)&data);
+    Py_EndInterpreter(tstate);
+    PyThreadState_Swap(oldts);
+    if (res < 0) {
+        return NULL;
+    }
+    if (data.called == 0) {
+        PyErr_SetString(PyExc_RuntimeError, "atexit callback not called");
+        return NULL;
+    }
+    Py_RETURN_NONE;
+}
+
+
 static PyObject *test_buildvalue_issue38913(PyObject *, PyObject *);
 
 static PyMethodDef TestMethods[] = {
@@ -3525,6 +3556,7 @@ static PyMethodDef TestMethods[] = {
     {"function_set_kw_defaults", function_set_kw_defaults, METH_VARARGS, NULL},
     {"test_gc_visit_objects_basic", test_gc_visit_objects_basic, METH_NOARGS, NULL},
     {"test_gc_visit_objects_exit_early", test_gc_visit_objects_exit_early, METH_NOARGS, NULL},
+    {"test_atexit", test_atexit, METH_NOARGS},
     {NULL, NULL} /* sentinel */
 };
 

Modules/_xxinterpchannelsmodule.c

@@ -174,19 +174,7 @@ _release_xid_data(_PyCrossInterpreterData *data, int ignoreexc)
     }
     int res = _PyCrossInterpreterData_Release(data);
     if (res < 0) {
-        // XXX Fix this!
+        /* The owning interpreter is already destroyed. */
-        /* The owning interpreter is already destroyed.
-         * Ideally, this shouldn't ever happen.  When an interpreter is
-         * about to be destroyed, we should clear out all of its objects
-         * from every channel associated with that interpreter.
-         * For now we hack around that to resolve refleaks, by decref'ing
-         * the released object here, even if its the wrong interpreter.
-         * The owning interpreter has already been destroyed
-         * so we should be okay, especially since the currently
-         * shareable types are all very basic, with no GC.
-         * That said, it becomes much messier once interpreters
-         * no longer share a GIL, so this needs to be fixed before then. */
-        _PyCrossInterpreterData_Clear(NULL, data);
         if (ignoreexc) {
             // XXX Emit a warning?
             PyErr_Clear();
@@ -489,6 +477,30 @@ _channelqueue_get(_channelqueue *queue)
     return _channelitem_popped(item);
 }
 
+static void
+_channelqueue_drop_interpreter(_channelqueue *queue, int64_t interp)
+{
+    _channelitem *prev = NULL;
+    _channelitem *next = queue->first;
+    while (next != NULL) {
+        _channelitem *item = next;
+        next = item->next;
+        if (item->data->interp == interp) {
+            if (prev == NULL) {
+                queue->first = item->next;
+            }
+            else {
+                prev->next = item->next;
+            }
+            _channelitem_free(item);
+            queue->count -= 1;
+        }
+        else {
+            prev = item;
+        }
+    }
+}
+
 /* channel-interpreter associations */
 
 struct _channelend;
@@ -693,6 +705,20 @@ _channelends_close_interpreter(_channelends *ends, int64_t interp, int which)
     return 0;
 }
 
+static void
+_channelends_drop_interpreter(_channelends *ends, int64_t interp)
+{
+    _channelend *end;
+    end = _channelend_find(ends->send, interp, NULL);
+    if (end != NULL) {
+        _channelends_close_end(ends, end, 1);
+    }
+    end = _channelend_find(ends->recv, interp, NULL);
+    if (end != NULL) {
+        _channelends_close_end(ends, end, 0);
+    }
+}
+
 static void
 _channelends_close_all(_channelends *ends, int which, int force)
 {
@@ -841,6 +867,18 @@ done:
     return res;
 }
 
+static void
+_channel_drop_interpreter(_PyChannelState *chan, int64_t interp)
+{
+    PyThread_acquire_lock(chan->mutex, WAIT_LOCK);
+
+    _channelqueue_drop_interpreter(chan->queue, interp);
+    _channelends_drop_interpreter(chan->ends, interp);
+    chan->open = _channelends_is_open(chan->ends);
+
+    PyThread_release_lock(chan->mutex);
+}
+
 static int
 _channel_close_all(_PyChannelState *chan, int end, int force)
 {
@@ -1213,6 +1251,21 @@ done:
     return cids;
 }
 
+static void
+_channels_drop_interpreter(_channels *channels, int64_t interp)
+{
+    PyThread_acquire_lock(channels->mutex, WAIT_LOCK);
+
+    _channelref *ref = channels->head;
+    for (; ref != NULL; ref = ref->next) {
+        if (ref->chan != NULL) {
+            _channel_drop_interpreter(ref->chan, interp);
+        }
+    }
+
+    PyThread_release_lock(channels->mutex);
+}
+
 /* support for closing non-empty channels */
 
 struct _channel_closing {
@@ -1932,6 +1985,19 @@ _global_channels(void) {
 }
 
 
+static void
+clear_interpreter(void *data)
+{
+    if (_globals.module_count == 0) {
+        return;
+    }
+    PyInterpreterState *interp = (PyInterpreterState *)data;
+    assert(interp == _get_current_interp());
+    int64_t id = PyInterpreterState_GetID(interp);
+    _channels_drop_interpreter(&_globals.channels, id);
+}
+
+
 static PyObject *
 channel_create(PyObject *self, PyObject *Py_UNUSED(ignored))
 {
@@ -2339,6 +2405,10 @@ module_exec(PyObject *mod)
         goto error;
     }
 
+    // Make sure chnnels drop objects owned by this interpreter
+    PyInterpreterState *interp = _get_current_interp();
+    _Py_AtExit(interp, clear_interpreter, (void *)interp);
+
     return 0;
 
 error:

Modules/_xxsubinterpretersmodule.c

@@ -67,16 +67,7 @@ _release_xid_data(_PyCrossInterpreterData *data, int ignoreexc)
     }
     int res = _PyCrossInterpreterData_Release(data);
     if (res < 0) {
-        // XXX Fix this!
+        /* The owning interpreter is already destroyed. */
-        /* The owning interpreter is already destroyed.
-         * Ideally, this shouldn't ever happen.  (It's highly unlikely.)
-         * For now we hack around that to resolve refleaks, by decref'ing
-         * the released object here, even if its the wrong interpreter.
-         * The owning interpreter has already been destroyed
-         * so we should be okay, especially since the currently
-         * shareable types are all very basic, with no GC.
-         * That said, it becomes much messier once interpreters
-         * no longer share a GIL, so this needs to be fixed before then. */
         _PyCrossInterpreterData_Clear(NULL, data);
         if (ignoreexc) {
             // XXX Emit a warning?

Modules/atexitmodule.c

@@ -7,6 +7,7 @@
  */
 
 #include "Python.h"
+#include "pycore_atexit.h"
 #include "pycore_initconfig.h"    // _PyStatus_NO_MEMORY
 #include "pycore_interp.h"        // PyInterpreterState.atexit
 #include "pycore_pystate.h"       // _PyInterpreterState_GET
@@ -22,10 +23,36 @@ get_atexit_state(void)
 }
 
 
+int
+_Py_AtExit(PyInterpreterState *interp,
+           atexit_datacallbackfunc func, void *data)
+{
+    assert(interp == _PyInterpreterState_GET());
+    atexit_callback *callback = PyMem_Malloc(sizeof(atexit_callback));
+    if (callback == NULL) {
+        PyErr_NoMemory();
+        return -1;
+    }
+    callback->func = func;
+    callback->data = data;
+    callback->next = NULL;
+
+    struct atexit_state *state = &interp->atexit;
+    if (state->ll_callbacks == NULL) {
+        state->ll_callbacks = callback;
+        state->last_ll_callback = callback;
+    }
+    else {
+        state->last_ll_callback->next = callback;
+    }
+    return 0;
+}
+
+
 static void
 atexit_delete_cb(struct atexit_state *state, int i)
 {
-    atexit_callback *cb = state->callbacks[i];
+    atexit_py_callback *cb = state->callbacks[i];
     state->callbacks[i] = NULL;
 
     Py_DECREF(cb->func);
@@ -39,7 +66,7 @@ atexit_delete_cb(struct atexit_state *state, int i)
 static void
 atexit_cleanup(struct atexit_state *state)
 {
-    atexit_callback *cb;
+    atexit_py_callback *cb;
     for (int i = 0; i < state->ncallbacks; i++) {
         cb = state->callbacks[i];
         if (cb == NULL)
@@ -60,7 +87,7 @@ _PyAtExit_Init(PyInterpreterState *interp)
 
     state->callback_len = 32;
     state->ncallbacks = 0;
-    state->callbacks = PyMem_New(atexit_callback*, state->callback_len);
+    state->callbacks = PyMem_New(atexit_py_callback*, state->callback_len);
     if (state->callbacks == NULL) {
         return _PyStatus_NO_MEMORY();
     }
@@ -75,6 +102,18 @@ _PyAtExit_Fini(PyInterpreterState *interp)
     atexit_cleanup(state);
     PyMem_Free(state->callbacks);
     state->callbacks = NULL;
+
+    atexit_callback *next = state->ll_callbacks;
+    state->ll_callbacks = NULL;
+    while (next != NULL) {
+        atexit_callback *callback = next;
+        next = callback->next;
+        atexit_datacallbackfunc exitfunc = callback->func;
+        void *data = callback->data;
+        // It was allocated in _PyAtExit_AddCallback().
+        PyMem_Free(callback);
+        exitfunc(data);
+    }
 }
 
 
@@ -88,7 +127,7 @@ atexit_callfuncs(struct atexit_state *state)
     }
 
     for (int i = state->ncallbacks - 1; i >= 0; i--) {
-        atexit_callback *cb = state->callbacks[i];
+        atexit_py_callback *cb = state->callbacks[i];
         if (cb == NULL) {
             continue;
         }
@@ -152,17 +191,17 @@ atexit_register(PyObject *module, PyObject *args, PyObject *kwargs)
 
     struct atexit_state *state = get_atexit_state();
     if (state->ncallbacks >= state->callback_len) {
-        atexit_callback **r;
+        atexit_py_callback **r;
         state->callback_len += 16;
-        size_t size = sizeof(atexit_callback*) * (size_t)state->callback_len;
+        size_t size = sizeof(atexit_py_callback*) * (size_t)state->callback_len;
-        r = (atexit_callback**)PyMem_Realloc(state->callbacks, size);
+        r = (atexit_py_callback**)PyMem_Realloc(state->callbacks, size);
         if (r == NULL) {
             return PyErr_NoMemory();
         }
         state->callbacks = r;
     }
 
-    atexit_callback *callback = PyMem_Malloc(sizeof(atexit_callback));
+    atexit_py_callback *callback = PyMem_Malloc(sizeof(atexit_py_callback));
     if (callback == NULL) {
         return PyErr_NoMemory();
     }
@@ -233,7 +272,7 @@ atexit_unregister(PyObject *module, PyObject *func)
     struct atexit_state *state = get_atexit_state();
     for (int i = 0; i < state->ncallbacks; i++)
     {
-        atexit_callback *cb = state->callbacks[i];
+        atexit_py_callback *cb = state->callbacks[i];
         if (cb == NULL) {
             continue;
         }

PCbuild/pythoncore.vcxproj

@@ -197,6 +197,7 @@
     <ClInclude Include="..\Include\internal\pycore_asdl.h" />
     <ClInclude Include="..\Include\internal\pycore_ast.h" />
     <ClInclude Include="..\Include\internal\pycore_ast_state.h" />
+    <ClInclude Include="..\Include\internal\pycore_atexit.h" />
     <ClInclude Include="..\Include\internal\pycore_atomic.h" />
     <ClInclude Include="..\Include\internal\pycore_atomic_funcs.h" />
     <ClInclude Include="..\Include\internal\pycore_bitutils.h" />

PCbuild/pythoncore.vcxproj.filters

@@ -498,6 +498,9 @@
     <ClInclude Include="..\Include\internal\pycore_ast_state.h">
       <Filter>Include\internal</Filter>
     </ClInclude>
+    <ClInclude Include="..\Include\internal\pycore_atexit.h">
+      <Filter>Include\internal</Filter>
+    </ClInclude>
     <ClInclude Include="..\Include\internal\pycore_atomic.h">
       <Filter>Include\internal</Filter>
     </ClInclude>

Python/pylifecycle.c

@@ -2937,23 +2937,23 @@ wait_for_thread_shutdown(PyThreadState *tstate)
     Py_DECREF(threading);
 }
 
-#define NEXITFUNCS 32
 int Py_AtExit(void (*func)(void))
 {
-    if (_PyRuntime.nexitfuncs >= NEXITFUNCS)
+    if (_PyRuntime.atexit.ncallbacks >= NEXITFUNCS)
         return -1;
-    _PyRuntime.exitfuncs[_PyRuntime.nexitfuncs++] = func;
+    _PyRuntime.atexit.callbacks[_PyRuntime.atexit.ncallbacks++] = func;
     return 0;
 }
 
 static void
 call_ll_exitfuncs(_PyRuntimeState *runtime)
 {
-    while (runtime->nexitfuncs > 0) {
+    struct _atexit_runtime_state *state = &runtime->atexit;
+    while (state->ncallbacks > 0) {
         /* pop last function from the list */
-        runtime->nexitfuncs--;
+        state->ncallbacks--;
-        void (*exitfunc)(void) = runtime->exitfuncs[runtime->nexitfuncs];
+        atexit_callbackfunc exitfunc = state->callbacks[state->ncallbacks];
-        runtime->exitfuncs[runtime->nexitfuncs] = NULL;
+        state->callbacks[state->ncallbacks] = NULL;
 
         exitfunc();
     }