From b325261e942d77732c4fef0dc3944d90a1e81538 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Wed, 9 Dec 2020 01:49:44 +0900 Subject: [PATCH 1/5] bpo-40600: atexit only loads sinle time per interpreter --- Include/internal/pycore_interp.h | 1 + Modules/atexitmodule.c | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index 184878ce1460321..fd1bba236008964 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -212,6 +212,7 @@ struct _is { PyObject *codec_search_cache; PyObject *codec_error_registry; int codecs_initialized; + int atexit_initialized; PyConfig config; #ifdef HAVE_DLOPEN diff --git a/Modules/atexitmodule.c b/Modules/atexitmodule.c index fd47ddd7731c5d8..f1bf19d9a9e8a42 100644 --- a/Modules/atexitmodule.c +++ b/Modules/atexitmodule.c @@ -7,6 +7,7 @@ */ #include "Python.h" +#include "pycore_interp.h" /* Forward declaration (for atexit_cleanup) */ static PyObject *atexit_clear(PyObject*, PyObject*); @@ -318,6 +319,10 @@ Two public functions, register and unregister, are defined.\n\ static int atexit_exec(PyObject *m) { atexitmodule_state *modstate; + PyInterpreterState *interp = PyInterpreterState_Get(); + if (interp->atexit_initialized == 1) { + return -1; + } modstate = get_atexit_state(m); modstate->callback_len = 32; @@ -328,6 +333,7 @@ atexit_exec(PyObject *m) { return -1; _Py_PyAtExit(atexit_callfuncs, m); + interp->atexit_initialized = 1; return 0; } From 6bf1fa160f8f91a70b52fe4bb3769bde5a2280d8 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Wed, 9 Dec 2020 01:55:20 +0900 Subject: [PATCH 2/5] bpo-40600: Add NEWS.d --- .../Core and Builtins/2020-12-09-01-55-10.bpo-40600.5pI5HG.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2020-12-09-01-55-10.bpo-40600.5pI5HG.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-12-09-01-55-10.bpo-40600.5pI5HG.rst b/Misc/NEWS.d/next/Core and Builtins/2020-12-09-01-55-10.bpo-40600.5pI5HG.rst new file mode 100644 index 000000000000000..0c6b3586c82c68b --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2020-12-09-01-55-10.bpo-40600.5pI5HG.rst @@ -0,0 +1,2 @@ +:mod:`atexit` supports only once loading per interpreter. Patch by Dong-hee +Na. From bbab42e92c06890a843a77f9f37a76eeeabba593 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Wed, 9 Dec 2020 02:01:15 +0900 Subject: [PATCH 3/5] bpo-40600: Update --- Modules/atexitmodule.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Modules/atexitmodule.c b/Modules/atexitmodule.c index f1bf19d9a9e8a42..4eeee6250200cdf 100644 --- a/Modules/atexitmodule.c +++ b/Modules/atexitmodule.c @@ -321,6 +321,8 @@ atexit_exec(PyObject *m) { atexitmodule_state *modstate; PyInterpreterState *interp = PyInterpreterState_Get(); if (interp->atexit_initialized == 1) { + PyErr_SetString(PyExc_ImportError, + "cannot load module more than once per interpreter"); return -1; } From eef1ce0a7e97437eab3cde63a7ece58ca502ba6d Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Wed, 9 Dec 2020 02:17:20 +0900 Subject: [PATCH 4/5] bpo-40600: Move the atexit list of callbacks into PyInterpreterState --- Include/internal/pycore_atexit.h | 26 +++++++ Include/internal/pycore_interp.h | 3 +- Include/internal/pycore_pylifecycle.h | 1 + Lib/test/test_atexit.py | 23 ++++++ Modules/atexitmodule.c | 104 ++++++++------------------ Python/pystate.c | 2 + 6 files changed, 84 insertions(+), 75 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 000000000000000..3f953b30223ee86 --- /dev/null +++ b/Include/internal/pycore_atexit.h @@ -0,0 +1,26 @@ +#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 struct { + PyObject *func; + PyObject *args; + PyObject *kwargs; +} _Py_atexit_callback; + +struct _atexit_runtime_state { + _Py_atexit_callback **atexit_callbacks; + int ncallbacks; + int callback_len; +}; + +#ifdef __cplusplus +} +#endif +#endif /* !Py_INTERNAL_ATEXIT_H */ \ No newline at end of file diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index fd1bba236008964..0bb0de5bab62c0d 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -13,6 +13,7 @@ extern "C" { #include "pycore_gil.h" // struct _gil_runtime_state #include "pycore_gc.h" // struct _gc_runtime_state #include "pycore_warnings.h" // struct _warnings_runtime_state +#include "pycore_atexit.h" // struct _atexit_runtime_state struct _pending_calls { PyThread_type_lock lock; @@ -212,7 +213,6 @@ struct _is { PyObject *codec_search_cache; PyObject *codec_error_registry; int codecs_initialized; - int atexit_initialized; PyConfig config; #ifdef HAVE_DLOPEN @@ -241,6 +241,7 @@ struct _is { uint64_t tstate_next_unique_id; struct _warnings_runtime_state warnings; + struct _atexit_runtime_state atexits; PyObject *audit_hooks; diff --git a/Include/internal/pycore_pylifecycle.h b/Include/internal/pycore_pylifecycle.h index b691e6325780ed3..c6badabffb0f781 100644 --- a/Include/internal/pycore_pylifecycle.h +++ b/Include/internal/pycore_pylifecycle.h @@ -85,6 +85,7 @@ extern void _PyHash_Fini(void); extern void _PyTraceMalloc_Fini(void); extern void _PyWarnings_Fini(PyInterpreterState *interp); extern void _PyAST_Fini(PyInterpreterState *interp); +extern void _PyAtExit_Fini(PyInterpreterState *interp); extern PyStatus _PyGILState_Init(PyThreadState *tstate); extern void _PyGILState_Fini(PyThreadState *tstate); diff --git a/Lib/test/test_atexit.py b/Lib/test/test_atexit.py index 3105f6c378193ef..c3a858a11853fa8 100644 --- a/Lib/test/test_atexit.py +++ b/Lib/test/test_atexit.py @@ -222,6 +222,29 @@ def callback(): self.assertEqual(os.read(r, len(expected)), expected) os.close(r) + def test_multiple_loading_on_subinterpreter(self): + expected = b"atexit1 callback\n" + r, w = os.pipe() + + code = r"""if 1: + import sys + import os + def callback(msg): + os.write({:d}, msg) + atexit1 = sys.modules.pop('atexit', None) + if atexit1 is None: + import atexit as atexit1 + del sys.modules['atexit'] + try: + import atexit as atexit2 + except ImportError: + pass + atexit1.register(callback, b"atexit1 callback\n") + """.format(w) + ret = support.run_in_subinterp(code) + os.close(w) + self.assertEqual(os.read(r, len(expected)), expected) + os.close(r) if __name__ == "__main__": unittest.main() diff --git a/Modules/atexitmodule.c b/Modules/atexitmodule.c index 4eeee6250200cdf..d9d3d9269c8f253 100644 --- a/Modules/atexitmodule.c +++ b/Modules/atexitmodule.c @@ -8,6 +8,10 @@ #include "Python.h" #include "pycore_interp.h" +#include "pycore_pystate.h" + +typedef struct _atexit_runtime_state atexitmodule_state; +typedef _Py_atexit_callback atexit_callback; /* Forward declaration (for atexit_cleanup) */ static PyObject *atexit_clear(PyObject*, PyObject*); @@ -17,24 +21,13 @@ static struct PyModuleDef atexitmodule; /* ===================================================================== */ /* Callback machinery. */ -typedef struct { - PyObject *func; - PyObject *args; - PyObject *kwargs; -} atexit_callback; - -typedef struct { - atexit_callback **atexit_callbacks; - int ncallbacks; - int callback_len; -} atexitmodule_state; static inline atexitmodule_state* -get_atexit_state(PyObject *module) +get_atexit_state() { - void *state = PyModule_GetState(module); - assert(state != NULL); - return (atexitmodule_state *)state; + PyInterpreterState *interp = _PyInterpreterState_GET(); + atexitmodule_state *modstate = &interp->atexits; + return modstate; } @@ -56,8 +49,7 @@ static void atexit_cleanup(atexitmodule_state *modstate) { atexit_callback *cb; - int i; - for (i = 0; i < modstate->ncallbacks; i++) { + for (int i = 0; i < modstate->ncallbacks; i++) { cb = modstate->atexit_callbacks[i]; if (cb == NULL) continue; @@ -79,7 +71,7 @@ atexit_callfuncs(PyObject *module) if (module == NULL) return; - modstate = get_atexit_state(module); + modstate = get_atexit_state(); if (modstate->ncallbacks == 0) return; @@ -137,7 +129,7 @@ atexit_register(PyObject *self, PyObject *args, PyObject *kwargs) atexit_callback *new_callback; PyObject *func = NULL; - modstate = get_atexit_state(self); + modstate = get_atexit_state(); if (modstate->ncallbacks >= modstate->callback_len) { atexit_callback **r; @@ -204,7 +196,7 @@ Clear the list of previously registered exit functions."); static PyObject * atexit_clear(PyObject *self, PyObject *unused) { - atexit_cleanup(get_atexit_state(self)); + atexit_cleanup(get_atexit_state()); Py_RETURN_NONE; } @@ -218,48 +210,11 @@ atexit_ncallbacks(PyObject *self, PyObject *unused) { atexitmodule_state *modstate; - modstate = get_atexit_state(self); + modstate = get_atexit_state(); return PyLong_FromSsize_t(modstate->ncallbacks); } -static int -atexit_m_traverse(PyObject *self, visitproc visit, void *arg) -{ - int i; - atexitmodule_state *modstate; - - modstate = (atexitmodule_state *)PyModule_GetState(self); - - for (i = 0; i < modstate->ncallbacks; i++) { - atexit_callback *cb = modstate->atexit_callbacks[i]; - if (cb == NULL) - continue; - Py_VISIT(cb->func); - Py_VISIT(cb->args); - Py_VISIT(cb->kwargs); - } - return 0; -} - -static int -atexit_m_clear(PyObject *self) -{ - atexitmodule_state *modstate; - modstate = (atexitmodule_state *)PyModule_GetState(self); - atexit_cleanup(modstate); - return 0; -} - -static void -atexit_free(PyObject *m) -{ - atexitmodule_state *modstate; - modstate = (atexitmodule_state *)PyModule_GetState(m); - atexit_cleanup(modstate); - PyMem_Free(modstate->atexit_callbacks); -} - PyDoc_STRVAR(atexit_unregister__doc__, "unregister(func) -> None\n\ \n\ @@ -275,7 +230,7 @@ atexit_unregister(PyObject *self, PyObject *func) atexit_callback *cb; int i, eq; - modstate = get_atexit_state(self); + modstate = get_atexit_state(); for (i = 0; i < modstate->ncallbacks; i++) { @@ -318,24 +273,21 @@ Two public functions, register and unregister, are defined.\n\ static int atexit_exec(PyObject *m) { - atexitmodule_state *modstate; - PyInterpreterState *interp = PyInterpreterState_Get(); - if (interp->atexit_initialized == 1) { + atexitmodule_state *modstate = get_atexit_state(); + if (modstate->atexit_callbacks != NULL) { PyErr_SetString(PyExc_ImportError, "cannot load module more than once per interpreter"); return -1; } - - modstate = get_atexit_state(m); modstate->callback_len = 32; modstate->ncallbacks = 0; modstate->atexit_callbacks = PyMem_New(atexit_callback*, modstate->callback_len); - if (modstate->atexit_callbacks == NULL) + if (modstate->atexit_callbacks == NULL) { return -1; + } _Py_PyAtExit(atexit_callfuncs, m); - interp->atexit_initialized = 1; return 0; } @@ -346,16 +298,20 @@ static PyModuleDef_Slot atexit_slots[] = { static struct PyModuleDef atexitmodule = { PyModuleDef_HEAD_INIT, - "atexit", - atexit__doc__, - sizeof(atexitmodule_state), - atexit_methods, - atexit_slots, - atexit_m_traverse, - atexit_m_clear, - (freefunc)atexit_free + .m_name = "atexit", + .m_doc = atexit__doc__, + .m_methods = atexit_methods, + .m_slots = atexit_slots, }; +void +_PyAtExit_Fini(PyInterpreterState *interp) +{ + atexitmodule_state *modstate = &interp->atexits; + atexit_cleanup(modstate); + PyMem_Free(modstate->atexit_callbacks); +} + PyMODINIT_FUNC PyInit_atexit(void) { diff --git a/Python/pystate.c b/Python/pystate.c index 8da583f8e06bc0c..47ab300dd636463 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -9,6 +9,7 @@ #include "pycore_pymem.h" // _PyMem_SetDefaultAllocator() #include "pycore_pystate.h" // _PyThreadState_GET() #include "pycore_sysmodule.h" +#include "pycore_atexit.h" /* -------------------------------------------------------------------------- CAUTION @@ -303,6 +304,7 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate) _PyAST_Fini(interp); _PyWarnings_Fini(interp); + _PyAtExit_Fini(interp); // All Python types must be destroyed before the last GC collection. Python // types create a reference cycle to themselves in their in their From 6959ca463a767d924ebae3af67cb6b0a61e2bde8 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 11 Dec 2020 02:52:46 +0900 Subject: [PATCH 5/5] bpo-40600: fix --- Modules/atexitmodule.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Modules/atexitmodule.c b/Modules/atexitmodule.c index d9d3d9269c8f253..0970830bf3b3d2e 100644 --- a/Modules/atexitmodule.c +++ b/Modules/atexitmodule.c @@ -272,7 +272,8 @@ Two public functions, register and unregister, are defined.\n\ "); static int -atexit_exec(PyObject *m) { +atexit_exec(PyObject *m) +{ atexitmodule_state *modstate = get_atexit_state(); if (modstate->atexit_callbacks != NULL) { PyErr_SetString(PyExc_ImportError,