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.)

Author: ericsnowcurrently

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
-    void (*exitfuncs[NEXITFUNCS])(void);
-    int nexitfuncs;
+    struct _atexit_runtime_state atexit;
 
     struct _import_runtime_state imports;
     struct _ceval_runtime_state ceval;

Lib/test/test__xxinterpchannels.py

@@ -550,6 +550,7 @@ def test_channel_list_interpreters_closed_send_end(self):
             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 @@ def test_recv_default(self):
         self.assertIs(obj6, default)
 
     def test_recv_sending_interp_destroyed(self):
-        cid = channels.create()
-        interp = interpreters.create()
-        interpreters.run_string(interp, dedent(f"""
-            import _xxinterpchannels as _channels
-            _channels.send({cid}, b'spam')
-            """))
-        interpreters.destroy(interp)
-
-        with self.assertRaisesRegex(RuntimeError,
-                                    'unrecognized interpreter ID'):
-            channels.recv(cid)
+        with self.subTest('closed'):
+            cid1 = channels.create()
+            interp = interpreters.create()
+            interpreters.run_string(interp, dedent(f"""
+                import _xxinterpchannels as _channels
+                _channels.send({cid1}, b'spam')
+                """))
+            interpreters.destroy(interp)
+
+            with self.assertRaisesRegex(RuntimeError,
+                                        f'channel {cid1} is closed'):
+                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.
-         * 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);
+        /* The owning interpreter is already destroyed. */
         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 @@ _channel_close_interpreter(_PyChannelState *chan, int64_t interp, int end)
     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 @@ _channels_list_all(_channels *channels, int64_t *count)
     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.
-         * 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. */
+        /* The owning interpreter is already destroyed. */
         _PyCrossInterpreterData_Clear(NULL, data);
         if (ignoreexc) {
             // XXX Emit a warning?