From f5ae7107974ccd6cb942093f9a878635e2f7d327 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 14:33:29 -0600 Subject: [PATCH 01/10] atexit_callback -> atexit_py_callback. --- Include/internal/pycore_interp.h | 4 ++-- Modules/atexitmodule.c | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index 1f2c0db2eb5f27..ce0325a9e02d13 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -37,10 +37,10 @@ typedef struct { PyObject *func; PyObject *args; PyObject *kwargs; -} atexit_callback; +} atexit_py_callback; struct atexit_state { - atexit_callback **callbacks; + atexit_py_callback **callbacks; int ncallbacks; int callback_len; }; diff --git a/Modules/atexitmodule.c b/Modules/atexitmodule.c index a1c511e09d704e..00b186fc15015e 100644 --- a/Modules/atexitmodule.c +++ b/Modules/atexitmodule.c @@ -25,7 +25,7 @@ get_atexit_state(void) 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 +39,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 +60,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(); } @@ -88,7 +88,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 +152,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 +233,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; } From e6d4776ad1861b05bf3b16ac74151ee33a583c83 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 14:50:44 -0600 Subject: [PATCH 02/10] Add pycore_atexit.h. --- Include/internal/pycore_atexit.h | 39 ++++++++++++++++++++++++++++++ Include/internal/pycore_interp.h | 17 ++----------- Include/internal/pycore_runtime.h | 5 ++-- Makefile.pre.in | 1 + Modules/atexitmodule.c | 1 + PCbuild/pythoncore.vcxproj | 1 + PCbuild/pythoncore.vcxproj.filters | 3 +++ Python/pylifecycle.c | 14 +++++------ 8 files changed, 56 insertions(+), 25 deletions(-) create mode 100644 Include/internal/pycore_atexit.h diff --git a/Include/internal/pycore_atexit.h b/Include/internal/pycore_atexit.h new file mode 100644 index 00000000000000..1f1c3a1c3ce0fa --- /dev/null +++ b/Include/internal/pycore_atexit.h @@ -0,0 +1,39 @@ +#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 + + +typedef void (*atexit_callbackfunc)(void); + + +typedef struct { + PyObject *func; + PyObject *args; + PyObject *kwargs; +} atexit_py_callback; + +struct atexit_state { + atexit_py_callback **callbacks; + int ncallbacks; + int callback_len; +}; + + + +struct _atexit_runtime_state { +#define NEXITFUNCS 32 + atexit_callbackfunc callbacks[NEXITFUNCS]; + int ncallbacks; +}; + + +#ifdef __cplusplus +} +#endif +#endif /* !Py_INTERNAL_ATEXIT_H */ diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index ce0325a9e02d13..d64a68cd2da54e 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -10,8 +10,9 @@ extern "C" { #include -#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_py_callback; - -struct atexit_state { - atexit_py_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 8877b282bc38de..3ebe49926edda6 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/Makefile.pre.in b/Makefile.pre.in index 9fdbd8db19bb33..1c1bddcad82475 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/atexitmodule.c b/Modules/atexitmodule.c index 00b186fc15015e..e8e964f75155ae 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 diff --git a/PCbuild/pythoncore.vcxproj b/PCbuild/pythoncore.vcxproj index 8fab600334160d..29f32db579fa40 100644 --- a/PCbuild/pythoncore.vcxproj +++ b/PCbuild/pythoncore.vcxproj @@ -197,6 +197,7 @@ + diff --git a/PCbuild/pythoncore.vcxproj.filters b/PCbuild/pythoncore.vcxproj.filters index 6c5d8dd89f5bc7..6a622fd93930ad 100644 --- a/PCbuild/pythoncore.vcxproj.filters +++ b/PCbuild/pythoncore.vcxproj.filters @@ -498,6 +498,9 @@ Include\internal + + Include\internal + Include\internal diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 8110d94ba17526..d6627bc6b7e86b 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(); } From c719f0214891d0a0dceee30d27a00cf9f7719694 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 15:55:30 -0600 Subject: [PATCH 03/10] Add _Py_AtExit(). --- Include/internal/pycore_atexit.h | 33 ++++++++++++++++++++++------ Modules/_testinternalcapi.c | 33 ++++++++++++++++++++++++++++ Modules/atexitmodule.c | 37 ++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 7 deletions(-) diff --git a/Include/internal/pycore_atexit.h b/Include/internal/pycore_atexit.h index 1f1c3a1c3ce0fa..994d9c8148a3e3 100644 --- a/Include/internal/pycore_atexit.h +++ b/Include/internal/pycore_atexit.h @@ -9,8 +9,29 @@ extern "C" { #endif +//############### +// runtime atexit + typedef void (*atexit_callbackfunc)(void); +struct _atexit_runtime_state { +#define NEXITFUNCS 32 + atexit_callbackfunc callbacks[NEXITFUNCS]; + int ncallbacks; +}; + + +//################### +// interpreter atexit + +typedef void (*atexit_datacallbackfunc)(void *); + +struct atexit_callback; +typedef struct atexit_callback { + atexit_datacallbackfunc func; + void *data; + struct atexit_callback *next; +} atexit_callback; typedef struct { PyObject *func; @@ -22,16 +43,14 @@ struct atexit_state { atexit_py_callback **callbacks; int ncallbacks; int callback_len; -}; - - -struct _atexit_runtime_state { -#define NEXITFUNCS 32 - atexit_callbackfunc callbacks[NEXITFUNCS]; - int ncallbacks; + atexit_callback *ll_callbacks; + atexit_callback *last_ll_callback; }; +PyAPI_FUNC(int) _Py_AtExit( + PyInterpreterState *, atexit_datacallbackfunc, void *); + #ifdef __cplusplus } diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 632fac2de0c419..47a41c8eedf185 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -12,6 +12,7 @@ #define PY_SSIZE_T_CLEAN #include "Python.h" +#include "pycore_atexit.h" // _Py_AtExit() #include "pycore_atomic_funcs.h" // _Py_atomic_int_get() #include "pycore_bitutils.h" // _Py_bswap32() #include "pycore_compile.h" // _PyCompile_CodeGen, _PyCompile_OptimizeCfg @@ -685,6 +686,37 @@ clear_extension(PyObject *self, PyObject *args) } +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 PyMethodDef module_functions[] = { {"get_configs", get_configs, METH_NOARGS}, {"get_recursion_depth", get_recursion_depth, METH_NOARGS}, @@ -707,6 +739,7 @@ static PyMethodDef module_functions[] = { _TESTINTERNALCAPI_OPTIMIZE_CFG_METHODDEF {"get_interp_settings", get_interp_settings, METH_VARARGS, NULL}, {"clear_extension", clear_extension, METH_VARARGS, NULL}, + {"test_atexit", test_atexit, METH_NOARGS}, {NULL, NULL} /* sentinel */ }; diff --git a/Modules/atexitmodule.c b/Modules/atexitmodule.c index e8e964f75155ae..5f78eeb87acdcd 100644 --- a/Modules/atexitmodule.c +++ b/Modules/atexitmodule.c @@ -23,6 +23,31 @@ get_atexit_state(void) } +int +_Py_AtExit(PyInterpreterState *interp, + atexit_datacallbackfunc func, void *data) +{ + 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) { @@ -76,6 +101,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); + } } From 47c302d459fff4d16ebf1f72fb1ae7c835d27fc9 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 16:04:20 -0600 Subject: [PATCH 04/10] Add a TODO comment. --- Include/internal/pycore_atexit.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Include/internal/pycore_atexit.h b/Include/internal/pycore_atexit.h index 994d9c8148a3e3..8b569422bc44bb 100644 --- a/Include/internal/pycore_atexit.h +++ b/Include/internal/pycore_atexit.h @@ -40,12 +40,15 @@ typedef struct { } 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; - - atexit_callback *ll_callbacks; - atexit_callback *last_ll_callback; }; PyAPI_FUNC(int) _Py_AtExit( From aaeaaa6b1d8aeed439f598298291b4650aa4224a Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 16:13:24 -0600 Subject: [PATCH 05/10] Move _Py_AtExit() to the public API. --- Include/cpython/pylifecycle.h | 4 ++++ Include/internal/pycore_atexit.h | 5 ----- Modules/_testcapimodule.c | 32 +++++++++++++++++++++++++++++++ Modules/_testinternalcapi.c | 33 -------------------------------- 4 files changed, 36 insertions(+), 38 deletions(-) diff --git a/Include/cpython/pylifecycle.h b/Include/cpython/pylifecycle.h index 821b169d7a1759..79d55711319e55 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 index 8b569422bc44bb..b4663b396852f3 100644 --- a/Include/internal/pycore_atexit.h +++ b/Include/internal/pycore_atexit.h @@ -24,8 +24,6 @@ struct _atexit_runtime_state { //################### // interpreter atexit -typedef void (*atexit_datacallbackfunc)(void *); - struct atexit_callback; typedef struct atexit_callback { atexit_datacallbackfunc func; @@ -51,9 +49,6 @@ struct atexit_state { int callback_len; }; -PyAPI_FUNC(int) _Py_AtExit( - PyInterpreterState *, atexit_datacallbackfunc, void *); - #ifdef __cplusplus } diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 3d9a2aeeb7cfd5..557a6d46ed4632 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/_testinternalcapi.c b/Modules/_testinternalcapi.c index 47a41c8eedf185..632fac2de0c419 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -12,7 +12,6 @@ #define PY_SSIZE_T_CLEAN #include "Python.h" -#include "pycore_atexit.h" // _Py_AtExit() #include "pycore_atomic_funcs.h" // _Py_atomic_int_get() #include "pycore_bitutils.h" // _Py_bswap32() #include "pycore_compile.h" // _PyCompile_CodeGen, _PyCompile_OptimizeCfg @@ -686,37 +685,6 @@ clear_extension(PyObject *self, PyObject *args) } -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 PyMethodDef module_functions[] = { {"get_configs", get_configs, METH_NOARGS}, {"get_recursion_depth", get_recursion_depth, METH_NOARGS}, @@ -739,7 +707,6 @@ static PyMethodDef module_functions[] = { _TESTINTERNALCAPI_OPTIMIZE_CFG_METHODDEF {"get_interp_settings", get_interp_settings, METH_VARARGS, NULL}, {"clear_extension", clear_extension, METH_VARARGS, NULL}, - {"test_atexit", test_atexit, METH_NOARGS}, {NULL, NULL} /* sentinel */ }; From b5396e421d1bcc6f92bfc36d4aa61a9b33f18c61 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 16:22:01 -0600 Subject: [PATCH 06/10] Test a constraint. --- Modules/atexitmodule.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Modules/atexitmodule.c b/Modules/atexitmodule.c index 5f78eeb87acdcd..47afd7f0751039 100644 --- a/Modules/atexitmodule.c +++ b/Modules/atexitmodule.c @@ -27,6 +27,7 @@ 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(); From 448b48a9c1f44cf0b261bf66bacce9cef2c5b9c0 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 17:34:03 -0600 Subject: [PATCH 07/10] Add an atexit callback for _xxinterpchannels. --- Modules/_xxinterpchannelsmodule.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Modules/_xxinterpchannelsmodule.c b/Modules/_xxinterpchannelsmodule.c index ef1cdcab952271..2f7d8e9b4b491a 100644 --- a/Modules/_xxinterpchannelsmodule.c +++ b/Modules/_xxinterpchannelsmodule.c @@ -1932,6 +1932,12 @@ _global_channels(void) { } +static void +clear_interpreter(void *data) +{ +} + + static PyObject * channel_create(PyObject *self, PyObject *Py_UNUSED(ignored)) { @@ -2339,6 +2345,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: From c86f7380047394a72a19edd8249d2860041df0f3 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 17:52:24 -0600 Subject: [PATCH 08/10] Implement the callback. --- Lib/test/test__xxinterpchannels.py | 40 ++++++++++++----- Modules/_xxinterpchannelsmodule.c | 72 ++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 11 deletions(-) diff --git a/Lib/test/test__xxinterpchannels.py b/Lib/test/test__xxinterpchannels.py index 69bda89a1688f5..b65281106f667c 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/Modules/_xxinterpchannelsmodule.c b/Modules/_xxinterpchannelsmodule.c index 2f7d8e9b4b491a..4756402949d249 100644 --- a/Modules/_xxinterpchannelsmodule.c +++ b/Modules/_xxinterpchannelsmodule.c @@ -489,6 +489,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 +717,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 +879,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 +1263,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 { @@ -1935,6 +2000,13 @@ _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); } From 1827feb2e7d866b1ef71e1398b1e07425dd12aba Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 17:56:33 -0600 Subject: [PATCH 09/10] Drop the _PyCrossInterpreterData_Clear() call in _xxinterpchannels. --- Modules/_xxinterpchannelsmodule.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/Modules/_xxinterpchannelsmodule.c b/Modules/_xxinterpchannelsmodule.c index 4756402949d249..13b005eaef9866 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(); From 82b395cc4dc3ad498f1fcc35eebacd02024d5e47 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 5 Apr 2023 17:59:39 -0600 Subject: [PATCH 10/10] Drop the _PyCrossInterpreterData_Clear() call in _xxsubinterpreters. --- Modules/_xxsubinterpretersmodule.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c index 11164676c4d107..884fb0d31f2b7f 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?