From ad5cd09d8b9225756fc44d5bdae15e0b894c56d0 Mon Sep 17 00:00:00 2001 From: Marcel Plch Date: Tue, 7 Nov 2017 17:13:15 +0100 Subject: [PATCH 1/4] Change atexit behavior and PEP-489 multiphase init support. --- Doc/library/atexit.rst | 3 + Include/internal/pystate.h | 1 - Include/pylifecycle.h | 2 +- Include/pystate.h | 3 + Lib/test/test_embed.py | 5 ++ .../2017-11-28-15-04-14.bpo-31901.mDeCLK.rst | 1 + Modules/atexitmodule.c | 46 +++++----- Programs/_testembed.c | 84 ++++++++++++++++++- Python/pylifecycle.c | 33 +++++--- Python/pystate.c | 2 + 10 files changed, 144 insertions(+), 36 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2017-11-28-15-04-14.bpo-31901.mDeCLK.rst diff --git a/Doc/library/atexit.rst b/Doc/library/atexit.rst index 5c60b604c68301..c2c058e474cbdc 100644 --- a/Doc/library/atexit.rst +++ b/Doc/library/atexit.rst @@ -20,6 +20,9 @@ at interpreter termination time they will be run in the order ``C``, ``B``, program is killed by a signal not handled by Python, when a Python fatal internal error is detected, or when :func:`os._exit` is called. +.. versionchanged:: 3.7 + When used with C-API subinterpreters, registered functions + are local to the interpreter they were registered in. .. function:: register(func, *args, **kwargs) diff --git a/Include/internal/pystate.h b/Include/internal/pystate.h index 6b527fbe2144ee..d65ac2f96a6755 100644 --- a/Include/internal/pystate.h +++ b/Include/internal/pystate.h @@ -90,7 +90,6 @@ typedef struct pyruntimestate { #define NEXITFUNCS 32 void (*exitfuncs[NEXITFUNCS])(void); int nexitfuncs; - void (*pyexitfunc)(void); struct _gc_runtime_state gc; struct _warnings_runtime_state warnings; diff --git a/Include/pylifecycle.h b/Include/pylifecycle.h index 542306004d22a8..badd0bfbcb92ea 100644 --- a/Include/pylifecycle.h +++ b/Include/pylifecycle.h @@ -81,7 +81,7 @@ PyAPI_FUNC(void) Py_EndInterpreter(PyThreadState *); * exit functions. */ #ifndef Py_LIMITED_API -PyAPI_FUNC(void) _Py_PyAtExit(void (*func)(void)); +PyAPI_FUNC(void) _Py_PyAtExit(PyObject *, void (*func)(PyObject *)); #endif PyAPI_FUNC(int) Py_AtExit(void (*func)(void)); diff --git a/Include/pystate.h b/Include/pystate.h index c7ea179cf7dc99..f0de561ba9943a 100644 --- a/Include/pystate.h +++ b/Include/pystate.h @@ -113,6 +113,9 @@ typedef struct _is { PyObject *after_forkers_parent; PyObject *after_forkers_child; #endif + /* AtExit module */ + void (*pyexitfunc)(PyObject *); + PyObject *pyexitmodule; } PyInterpreterState; #endif /* !Py_LIMITED_API */ diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py index c7f45b59acc8a1..668f7241256c03 100644 --- a/Lib/test/test_embed.py +++ b/Lib/test/test_embed.py @@ -208,6 +208,11 @@ def test_bpo20891(self): self.assertEqual(out, '') self.assertEqual(err, '') + def test_atexit_callbacks(self): + out, err = self.run_embedded_interpreter("atexit_callbacks") + expected_output = "The test has passed." + self.assertEqual(out.strip(), expected_output) + if __name__ == "__main__": unittest.main() diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-11-28-15-04-14.bpo-31901.mDeCLK.rst b/Misc/NEWS.d/next/Core and Builtins/2017-11-28-15-04-14.bpo-31901.mDeCLK.rst new file mode 100644 index 00000000000000..782dd84d75a4c6 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2017-11-28-15-04-14.bpo-31901.mDeCLK.rst @@ -0,0 +1 @@ +The `atexit` module now has it's callback stored per interpreter. diff --git a/Modules/atexitmodule.c b/Modules/atexitmodule.c index 35ebf08ecd3c66..f96cafa9ca8bfd 100644 --- a/Modules/atexitmodule.c +++ b/Modules/atexitmodule.c @@ -63,15 +63,13 @@ atexit_cleanup(atexitmodule_state *modstate) /* Installed into pylifecycle.c's atexit mechanism */ static void -atexit_callfuncs(void) +atexit_callfuncs(PyObject *module) { PyObject *exc_type = NULL, *exc_value, *exc_tb, *r; atexit_callback *cb; - PyObject *module; atexitmodule_state *modstate; int i; - module = PyState_FindModule(&atexitmodule); if (module == NULL) return; modstate = GET_ATEXIT_STATE(module); @@ -185,7 +183,7 @@ Run all registered exit functions."); static PyObject * atexit_run_exitfuncs(PyObject *self, PyObject *unused) { - atexit_callfuncs(); + atexit_callfuncs(self); if (PyErr_Occurred()) return NULL; Py_RETURN_NONE; @@ -310,6 +308,26 @@ upon normal program termination.\n\ Two public functions, register and unregister, are defined.\n\ "); +static int +atexit_exec(PyObject *m) { + atexitmodule_state *modstate; + + 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) + return -1; + + _Py_PyAtExit(m, atexit_callfuncs); + return 0; +} + +static PyModuleDef_Slot atexit_slots[] = { + {Py_mod_exec, atexit_exec}, + {0, NULL} +}; static struct PyModuleDef atexitmodule = { PyModuleDef_HEAD_INIT, @@ -317,7 +335,7 @@ static struct PyModuleDef atexitmodule = { atexit__doc__, sizeof(atexitmodule_state), atexit_methods, - NULL, + atexit_slots, atexit_m_traverse, atexit_m_clear, (freefunc)atexit_free @@ -326,21 +344,5 @@ static struct PyModuleDef atexitmodule = { PyMODINIT_FUNC PyInit_atexit(void) { - PyObject *m; - atexitmodule_state *modstate; - - m = PyModule_Create(&atexitmodule); - if (m == NULL) - return NULL; - - 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) - return NULL; - - _Py_PyAtExit(atexit_callfuncs); - return m; + return PyModuleDef_Init(&atexitmodule); } diff --git a/Programs/_testembed.c b/Programs/_testembed.c index a528f3e3aa0af9..d2574db4bedf84 100644 --- a/Programs/_testembed.c +++ b/Programs/_testembed.c @@ -126,7 +126,6 @@ static int test_forced_io_encoding(void) return 0; } - /********************************************************* * Test parts of the C-API that work before initialization *********************************************************/ @@ -196,6 +195,88 @@ static int test_bpo20891(void) } +static int test_atexit_callbacks(void) { + PyObject *pName = NULL, *pModule = NULL, *pFunc = NULL; + PyObject *pArgs = NULL, *pValue = NULL; + PyObject *ax_name = NULL, *ax_module = NULL; + PyThreadState *subinterpret = NULL; + PyThreadState *mainState = NULL; + + _testembed_Py_Initialize(); + mainState = PyThreadState_Get(); + if (mainState == NULL) { + goto error; + } + subinterpret = Py_NewInterpreter(); + if (subinterpret == NULL) { + goto error; + } + PyThreadState_Swap(subinterpret); + pName = PyUnicode_FromString("builtins"); + if (pName == NULL) { + goto error; + } + ax_name = PyUnicode_FromString("atexit"); + if (ax_name == NULL) { + goto error; + } + + pModule = PyImport_Import(pName); + if (pModule == NULL) { + goto error; + } + ax_module = PyImport_Import(ax_name); + if (ax_module == NULL) { + goto error; + } + Py_CLEAR(pName); + Py_CLEAR(ax_name); + + pFunc = PyObject_GetAttrString(ax_module, "register"); + if (pFunc == NULL || !PyCallable_Check(pFunc)) { + goto error; + } + + pArgs = PyTuple_New(4); + if (pArgs == NULL) { + goto error; + } + PyTuple_SetItem(pArgs, 0, PyObject_GetAttrString(pModule, "exec")); + PyTuple_SetItem(pArgs, 1, PyUnicode_FromString("import sys;" + "print('The test has passed.');" + "sys.stdout.flush()")); + PyTuple_SetItem(pArgs, 2, PyDict_New()); + PyTuple_SetItem(pArgs, 3, PyDict_New()); + pValue = PyObject_CallObject(pFunc, pArgs); + if (pValue == NULL) { + goto error; + } + Py_CLEAR(pArgs); + Py_CLEAR(pValue); + Py_CLEAR(pFunc); + Py_CLEAR(pModule); + PyErr_Clear(); + Py_EndInterpreter(subinterpret); + PyThreadState_Swap(mainState); + if (Py_FinalizeEx() < 0) { + return -1; + } + return 0; + +error: + Py_XDECREF(pName); + Py_XDECREF(pModule); + Py_XDECREF(pFunc); + Py_XDECREF(pArgs); + Py_XDECREF(pValue); + Py_XDECREF(ax_name); + Py_XDECREF(ax_module); + Py_XDECREF(subinterpret); + Py_XDECREF(mainState); + Py_Finalize(); + return -1; +} + /* ********************************************************* * List of test cases and the function that implements it. * @@ -219,6 +300,7 @@ static struct TestCase TestCases[] = { { "repeated_init_and_subinterpreters", test_repeated_init_and_subinterpreters }, { "pre_initialization_api", test_pre_initialization_api }, { "bpo20891", test_bpo20891 }, + { "atexit_callbacks", test_atexit_callbacks }, { NULL, NULL } }; diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 2bac23d1cb066d..10e2fd38054ce0 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -56,7 +56,7 @@ static _PyInitError initfsencoding(PyInterpreterState *interp); static _PyInitError initsite(void); static _PyInitError init_sys_streams(PyInterpreterState *interp); static _PyInitError initsigs(void); -static void call_py_exitfuncs(void); +static void call_py_exitfuncs(PyInterpreterState *); static void wait_for_thread_shutdown(void); static void call_ll_exitfuncs(void); extern int _PyUnicode_Init(void); @@ -1088,6 +1088,10 @@ Py_FinalizeEx(void) wait_for_thread_shutdown(); + /* Get current thread state and interpreter pointer */ + tstate = PyThreadState_GET(); + interp = tstate->interp; + /* The interpreter is still entirely intact at this point, and the * exit funcs may be relying on that. In particular, if some thread * or exit func is still waiting to do an import, the import machinery @@ -1097,11 +1101,8 @@ Py_FinalizeEx(void) * threads created thru it, so this also protects pending imports in * the threads created via Threading. */ - call_py_exitfuncs(); - /* Get current thread state and interpreter pointer */ - tstate = PyThreadState_GET(); - interp = tstate->interp; + call_py_exitfuncs(interp); /* Copy the core config to be able to use it even after PyInterpreterState_Delete() */ @@ -1484,6 +1485,8 @@ Py_EndInterpreter(PyThreadState *tstate) wait_for_thread_shutdown(); + call_py_exitfuncs(interp); + if (tstate != interp->tstate_head || tstate->next != NULL) Py_FatalError("Py_EndInterpreter: not the last thread"); @@ -2095,20 +2098,28 @@ _Py_FatalInitError(_PyInitError err) # include "pythread.h" /* For the atexit module. */ -void _Py_PyAtExit(void (*func)(void)) +void _Py_PyAtExit(PyObject *module, void (*func)(PyObject *)) { + PyThreadState *ts; + PyInterpreterState *is; + + ts = PyThreadState_GET(); + is = ts->interp; + /* Guard against API misuse (see bpo-17852) */ - assert(_PyRuntime.pyexitfunc == NULL || _PyRuntime.pyexitfunc == func); - _PyRuntime.pyexitfunc = func; + assert(is->pyexitfunc == NULL || is->pyexitfunc == func); + + is->pyexitfunc = func; + is->pyexitmodule = module; } static void -call_py_exitfuncs(void) +call_py_exitfuncs(PyInterpreterState *istate) { - if (_PyRuntime.pyexitfunc == NULL) + if (istate->pyexitfunc == NULL) return; - (*_PyRuntime.pyexitfunc)(); + (*istate->pyexitfunc)(istate->pyexitmodule); PyErr_Clear(); } diff --git a/Python/pystate.c b/Python/pystate.c index 500f96768752f4..e9fd6e0461e0d0 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -153,6 +153,8 @@ PyInterpreterState_New(void) interp->after_forkers_parent = NULL; interp->after_forkers_child = NULL; #endif + interp->pyexitfunc = NULL; + interp->pyexitmodule = NULL; HEAD_LOCK(); interp->next = _PyRuntime.interpreters.head; From 11517cb1061e78273684d537880e6b9311b0fc14 Mon Sep 17 00:00:00 2001 From: Marcel Plch Date: Tue, 12 Dec 2017 16:44:15 +0100 Subject: [PATCH 2/4] fixes --- Include/pylifecycle.h | 2 +- Lib/test/test_atexit.py | 19 ++++++++++ Lib/test/test_embed.py | 5 --- Modules/atexitmodule.c | 2 +- Programs/_testembed.c | 83 ----------------------------------------- Python/pylifecycle.c | 2 +- 6 files changed, 22 insertions(+), 91 deletions(-) diff --git a/Include/pylifecycle.h b/Include/pylifecycle.h index badd0bfbcb92ea..19d8eb6eb31184 100644 --- a/Include/pylifecycle.h +++ b/Include/pylifecycle.h @@ -81,7 +81,7 @@ PyAPI_FUNC(void) Py_EndInterpreter(PyThreadState *); * exit functions. */ #ifndef Py_LIMITED_API -PyAPI_FUNC(void) _Py_PyAtExit(PyObject *, void (*func)(PyObject *)); +PyAPI_FUNC(void) _Py_PyAtExit(void (*func)(PyObject *), PyObject *); #endif PyAPI_FUNC(int) Py_AtExit(void (*func)(void)); diff --git a/Lib/test/test_atexit.py b/Lib/test/test_atexit.py index aa56388ef60881..3105f6c378193e 100644 --- a/Lib/test/test_atexit.py +++ b/Lib/test/test_atexit.py @@ -2,6 +2,7 @@ import unittest import io import atexit +import os from test import support from test.support import script_helper @@ -203,6 +204,24 @@ def f(): self.assertEqual(ret, 0) self.assertEqual(atexit._ncallbacks(), n) + def test_callback_on_subinterpreter_teardown(self): + # This tests if a callback is called on + # subinterpreter teardown. + expected = b"The test has passed!" + r, w = os.pipe() + + code = r"""if 1: + import os + import atexit + def callback(): + os.write({:d}, b"The test has passed!") + atexit.register(callback) + """.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/Lib/test/test_embed.py b/Lib/test/test_embed.py index 668f7241256c03..c7f45b59acc8a1 100644 --- a/Lib/test/test_embed.py +++ b/Lib/test/test_embed.py @@ -208,11 +208,6 @@ def test_bpo20891(self): self.assertEqual(out, '') self.assertEqual(err, '') - def test_atexit_callbacks(self): - out, err = self.run_embedded_interpreter("atexit_callbacks") - expected_output = "The test has passed." - self.assertEqual(out.strip(), expected_output) - if __name__ == "__main__": unittest.main() diff --git a/Modules/atexitmodule.c b/Modules/atexitmodule.c index f96cafa9ca8bfd..d11c280230d01d 100644 --- a/Modules/atexitmodule.c +++ b/Modules/atexitmodule.c @@ -320,7 +320,7 @@ atexit_exec(PyObject *m) { if (modstate->atexit_callbacks == NULL) return -1; - _Py_PyAtExit(m, atexit_callfuncs); + _Py_PyAtExit(atexit_callfuncs, m); return 0; } diff --git a/Programs/_testembed.c b/Programs/_testembed.c index d2574db4bedf84..b3d7aa442d1a7f 100644 --- a/Programs/_testembed.c +++ b/Programs/_testembed.c @@ -195,88 +195,6 @@ static int test_bpo20891(void) } -static int test_atexit_callbacks(void) { - PyObject *pName = NULL, *pModule = NULL, *pFunc = NULL; - PyObject *pArgs = NULL, *pValue = NULL; - PyObject *ax_name = NULL, *ax_module = NULL; - PyThreadState *subinterpret = NULL; - PyThreadState *mainState = NULL; - - _testembed_Py_Initialize(); - mainState = PyThreadState_Get(); - if (mainState == NULL) { - goto error; - } - subinterpret = Py_NewInterpreter(); - if (subinterpret == NULL) { - goto error; - } - PyThreadState_Swap(subinterpret); - pName = PyUnicode_FromString("builtins"); - if (pName == NULL) { - goto error; - } - ax_name = PyUnicode_FromString("atexit"); - if (ax_name == NULL) { - goto error; - } - - pModule = PyImport_Import(pName); - if (pModule == NULL) { - goto error; - } - ax_module = PyImport_Import(ax_name); - if (ax_module == NULL) { - goto error; - } - Py_CLEAR(pName); - Py_CLEAR(ax_name); - - pFunc = PyObject_GetAttrString(ax_module, "register"); - if (pFunc == NULL || !PyCallable_Check(pFunc)) { - goto error; - } - - pArgs = PyTuple_New(4); - if (pArgs == NULL) { - goto error; - } - PyTuple_SetItem(pArgs, 0, PyObject_GetAttrString(pModule, "exec")); - PyTuple_SetItem(pArgs, 1, PyUnicode_FromString("import sys;" - "print('The test has passed.');" - "sys.stdout.flush()")); - PyTuple_SetItem(pArgs, 2, PyDict_New()); - PyTuple_SetItem(pArgs, 3, PyDict_New()); - pValue = PyObject_CallObject(pFunc, pArgs); - if (pValue == NULL) { - goto error; - } - Py_CLEAR(pArgs); - Py_CLEAR(pValue); - Py_CLEAR(pFunc); - Py_CLEAR(pModule); - PyErr_Clear(); - Py_EndInterpreter(subinterpret); - PyThreadState_Swap(mainState); - if (Py_FinalizeEx() < 0) { - return -1; - } - return 0; - -error: - Py_XDECREF(pName); - Py_XDECREF(pModule); - Py_XDECREF(pFunc); - Py_XDECREF(pArgs); - Py_XDECREF(pValue); - Py_XDECREF(ax_name); - Py_XDECREF(ax_module); - Py_XDECREF(subinterpret); - Py_XDECREF(mainState); - Py_Finalize(); - return -1; -} - /* ********************************************************* * List of test cases and the function that implements it. * @@ -300,7 +218,6 @@ static struct TestCase TestCases[] = { { "repeated_init_and_subinterpreters", test_repeated_init_and_subinterpreters }, { "pre_initialization_api", test_pre_initialization_api }, { "bpo20891", test_bpo20891 }, - { "atexit_callbacks", test_atexit_callbacks }, { NULL, NULL } }; diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 10e2fd38054ce0..cab4dd869bcba8 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -2098,7 +2098,7 @@ _Py_FatalInitError(_PyInitError err) # include "pythread.h" /* For the atexit module. */ -void _Py_PyAtExit(PyObject *module, void (*func)(PyObject *)) +void _Py_PyAtExit(void (*func)(PyObject *), PyObject *module) { PyThreadState *ts; PyInterpreterState *is; From 1d83cc5d16f702c080f57830df20ec91361858d3 Mon Sep 17 00:00:00 2001 From: Marcel Plch Date: Thu, 14 Dec 2017 15:05:31 +0100 Subject: [PATCH 3/4] NULL modstate guards --- Modules/atexitmodule.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/Modules/atexitmodule.c b/Modules/atexitmodule.c index d11c280230d01d..afa1cfad6c4115 100644 --- a/Modules/atexitmodule.c +++ b/Modules/atexitmodule.c @@ -223,13 +223,15 @@ atexit_m_traverse(PyObject *self, visitproc visit, void *arg) atexitmodule_state *modstate; modstate = GET_ATEXIT_STATE(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); + if (modstate != NULL) { + 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; } @@ -239,7 +241,9 @@ atexit_m_clear(PyObject *self) { atexitmodule_state *modstate; modstate = GET_ATEXIT_STATE(self); - atexit_cleanup(modstate); + if (modstate != NULL) { + atexit_cleanup(modstate); + } return 0; } @@ -248,8 +252,10 @@ atexit_free(PyObject *m) { atexitmodule_state *modstate; modstate = GET_ATEXIT_STATE(m); - atexit_cleanup(modstate); - PyMem_Free(modstate->atexit_callbacks); + if (modstate != NULL) { + atexit_cleanup(modstate); + PyMem_Free(modstate->atexit_callbacks); + } } PyDoc_STRVAR(atexit_unregister__doc__, From b6ae20a54164887acff063592c22897d174303cc Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Tue, 19 Dec 2017 15:21:48 +0100 Subject: [PATCH 4/4] Fix typo in NEWS file --- .../Core and Builtins/2017-11-28-15-04-14.bpo-31901.mDeCLK.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-11-28-15-04-14.bpo-31901.mDeCLK.rst b/Misc/NEWS.d/next/Core and Builtins/2017-11-28-15-04-14.bpo-31901.mDeCLK.rst index 782dd84d75a4c6..613eee0bf581c4 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2017-11-28-15-04-14.bpo-31901.mDeCLK.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2017-11-28-15-04-14.bpo-31901.mDeCLK.rst @@ -1 +1 @@ -The `atexit` module now has it's callback stored per interpreter. +The `atexit` module now has its callback stored per interpreter.