[Python-checkins] gh-101659: Add _Py_AtExit() (gh-103298)
ericsnowcurrently
webhook-mailer at python.org
Wed Apr 5 20:42:09 EDT 2023
https://github.com/python/cpython/commit/03089fdccc7dbe3f69227fbd570df92278371e7f
commit: 03089fdccc7dbe3f69227fbd570df92278371e7f
branch: main
author: Eric Snow <ericsnowcurrently at gmail.com>
committer: ericsnowcurrently <ericsnowcurrently at gmail.com>
date: 2023-04-05T18:42:02-06:00
summary:
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.)
files:
A Include/internal/pycore_atexit.h
M Include/cpython/pylifecycle.h
M Include/internal/pycore_interp.h
M Include/internal/pycore_runtime.h
M Lib/test/test__xxinterpchannels.py
M Makefile.pre.in
M Modules/_testcapimodule.c
M Modules/_xxinterpchannelsmodule.c
M Modules/_xxsubinterpretersmodule.c
M Modules/atexitmodule.c
M PCbuild/pythoncore.vcxproj
M PCbuild/pythoncore.vcxproj.filters
M Python/pylifecycle.c
diff --git a/Include/cpython/pylifecycle.h b/Include/cpython/pylifecycle.h
index 821b169d7a17..79d55711319e 100644
--- a/Include/cpython/pylifecycle.h
+++ b/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 *);
diff --git a/Include/internal/pycore_atexit.h b/Include/internal/pycore_atexit.h
new file mode 100644
index 000000000000..b4663b396852
--- /dev/null
+++ b/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 */
diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h
index 1f2c0db2eb5f..d64a68cd2da5 100644
--- a/Include/internal/pycore_interp.h
+++ b/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;
};
diff --git a/Include/internal/pycore_runtime.h b/Include/internal/pycore_runtime.h
index 8877b282bc38..3ebe49926edd 100644
--- a/Include/internal/pycore_runtime.h
+++ b/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;
diff --git a/Lib/test/test__xxinterpchannels.py b/Lib/test/test__xxinterpchannels.py
index 69bda89a1688..b65281106f66 100644
--- a/Lib/test/test__xxinterpchannels.py
+++ b/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()
diff --git a/Makefile.pre.in b/Makefile.pre.in
index 9fdbd8db19bb..1c1bddcad824 100644
--- a/Makefile.pre.in
+++ b/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 \
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index 3d9a2aeeb7cf..557a6d46ed46 100644
--- a/Modules/_testcapimodule.c
+++ b/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 */
};
diff --git a/Modules/_xxinterpchannelsmodule.c b/Modules/_xxinterpchannelsmodule.c
index ef1cdcab9522..13b005eaef98 100644
--- a/Modules/_xxinterpchannelsmodule.c
+++ b/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:
diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c
index 11164676c4d1..884fb0d31f2b 100644
--- a/Modules/_xxsubinterpretersmodule.c
+++ b/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?
diff --git a/Modules/atexitmodule.c b/Modules/atexitmodule.c
index a1c511e09d70..47afd7f07510 100644
--- a/Modules/atexitmodule.c
+++ b/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;
- r = (atexit_callback**)PyMem_Realloc(state->callbacks, size);
+ size_t size = sizeof(atexit_py_callback*) * (size_t)state->callback_len;
+ 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;
}
diff --git a/PCbuild/pythoncore.vcxproj b/PCbuild/pythoncore.vcxproj
index 8fab60033416..29f32db579fa 100644
--- a/PCbuild/pythoncore.vcxproj
+++ b/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" />
diff --git a/PCbuild/pythoncore.vcxproj.filters b/PCbuild/pythoncore.vcxproj.filters
index 6c5d8dd89f5b..6a622fd93930 100644
--- a/PCbuild/pythoncore.vcxproj.filters
+++ b/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>
diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
index 8110d94ba175..d6627bc6b7e8 100644
--- a/Python/pylifecycle.c
+++ b/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--;
- void (*exitfunc)(void) = runtime->exitfuncs[runtime->nexitfuncs];
- runtime->exitfuncs[runtime->nexitfuncs] = NULL;
+ state->ncallbacks--;
+ atexit_callbackfunc exitfunc = state->callbacks[state->ncallbacks];
+ state->callbacks[state->ncallbacks] = NULL;
exitfunc();
}
More information about the Python-checkins
mailing list