From a97aafb9e8979d4d5af53fcc561f53e64ac38373 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 10 Dec 2024 13:34:45 +0000 Subject: [PATCH 1/7] Fix linked list in PyUnstable_AtExit --- Modules/atexitmodule.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Modules/atexitmodule.c b/Modules/atexitmodule.c index 297a8d74ba3bf4..6a4a5e7dd1e780 100644 --- a/Modules/atexitmodule.c +++ b/Modules/atexitmodule.c @@ -38,12 +38,13 @@ PyUnstable_AtExit(PyInterpreterState *interp, callback->next = NULL; struct atexit_state *state = &interp->atexit; - if (state->ll_callbacks == NULL) { + atexit_callback *top = state->ll_callbacks; + if (top == NULL) { state->ll_callbacks = callback; - state->last_ll_callback = callback; } else { - state->last_ll_callback->next = callback; + callback->next = top; + state->ll_callbacks = callback; } return 0; } From 2c471aa8923a1838e51c1205509ad14d0b55e577 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 10 Dec 2024 13:40:14 +0000 Subject: [PATCH 2/7] Remove dead field. --- Include/internal/pycore_atexit.h | 1 - 1 file changed, 1 deletion(-) diff --git a/Include/internal/pycore_atexit.h b/Include/internal/pycore_atexit.h index 507a5c03cbc792..cde5b530baf00c 100644 --- a/Include/internal/pycore_atexit.h +++ b/Include/internal/pycore_atexit.h @@ -44,7 +44,6 @@ typedef struct { 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. From 100f7801477e7da8b80dbba9a4bc9bcc15dbbbfe Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 10 Dec 2024 13:41:57 +0000 Subject: [PATCH 3/7] Move test to _testcapi --- Modules/_testcapimodule.c | 21 +++++++++++++++++++++ Modules/_testinternalcapi.c | 23 ----------------------- 2 files changed, 21 insertions(+), 23 deletions(-) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 26f68691e44f83..bdad0ba7fe32e8 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -3353,6 +3353,26 @@ type_freeze(PyObject *module, PyObject *args) Py_RETURN_NONE; } +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 = PyUnstable_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 TestMethods[] = { {"set_errno", set_errno, METH_VARARGS}, @@ -3495,6 +3515,7 @@ static PyMethodDef TestMethods[] = { {"test_critical_sections", test_critical_sections, METH_NOARGS}, {"finalize_thread_hang", finalize_thread_hang, METH_O, NULL}, {"type_freeze", type_freeze, METH_VARARGS}, + {"test_atexit", test_atexit, METH_NOARGS}, {NULL, NULL} /* sentinel */ }; diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 1bb71a3e80b39d..7c7db6c7fb243c 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -1247,28 +1247,6 @@ 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 = PyUnstable_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_pyobject_is_freed(const char *test_name, PyObject *op) { @@ -2128,7 +2106,6 @@ static PyMethodDef module_functions[] = { {"_PyTraceMalloc_GetTraceback", tracemalloc_get_traceback, METH_VARARGS}, {"test_tstate_capi", test_tstate_capi, METH_NOARGS, NULL}, {"_PyUnicode_TransformDecimalAndSpaceToASCII", unicode_transformdecimalandspacetoascii, METH_O}, - {"test_atexit", test_atexit, METH_NOARGS}, {"check_pyobject_forbidden_bytes_is_freed", check_pyobject_forbidden_bytes_is_freed, METH_NOARGS}, {"check_pyobject_freed_is_freed", check_pyobject_freed_is_freed, METH_NOARGS}, From b4105472f0b1fd8e421fc615b7cc556ab9c1c0fc Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 10 Dec 2024 13:47:05 +0000 Subject: [PATCH 4/7] Validate thread state and interpreter in the test. --- Modules/_testcapimodule.c | 37 ++++++++++++++++++++++++++++++++----- Modules/_testinternalcapi.c | 11 ----------- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index bdad0ba7fe32e8..8d86b535effb9a 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -3353,6 +3353,22 @@ type_freeze(PyObject *module, PyObject *args) Py_RETURN_NONE; } +struct atexit_data { + int called; + PyThreadState *tstate; + PyInterpreterState *interp; +}; + +static void +atexit_callback(void *data) +{ + struct atexit_data *at_data = (struct atexit_data *)data; + // Ensure that the callback is from the same interpreter + assert(PyThreadState_Get() == at_data->tstate); + assert(PyInterpreterState_Get() == at_data->interp); + ++at_data->called; +} + static PyObject * test_atexit(PyObject *self, PyObject *Py_UNUSED(args)) { @@ -3360,14 +3376,25 @@ test_atexit(PyObject *self, PyObject *Py_UNUSED(args)) PyThreadState *tstate = Py_NewInterpreter(); struct atexit_data data = {0}; - int res = PyUnstable_AtExit(tstate->interp, callback, (void *)&data); + data.tstate = PyThreadState_Get(); + data.interp = PyInterpreterState_Get(); + + int amount = 10; + for (int i = 0; i < amount; ++i) + { + int res = PyUnstable_AtExit(tstate->interp, atexit_callback, (void *)&data); + if (res < 0) { + Py_EndInterpreter(tstate); + PyThreadState_Swap(oldts); + PyErr_SetString(PyExc_RuntimeError, "atexit callback failed"); + return NULL; + } + } + Py_EndInterpreter(tstate); PyThreadState_Swap(oldts); - if (res < 0) { - return NULL; - } - if (data.called == 0) { + if (data.called != amount) { PyErr_SetString(PyExc_RuntimeError, "atexit callback not called"); return NULL; } diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 7c7db6c7fb243c..288daf09a5fe5c 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -1236,17 +1236,6 @@ unicode_transformdecimalandspacetoascii(PyObject *self, PyObject *arg) return _PyUnicode_TransformDecimalAndSpaceToASCII(arg); } - -struct atexit_data { - int called; -}; - -static void -callback(void *data) -{ - ((struct atexit_data *)data)->called += 1; -} - static PyObject * test_pyobject_is_freed(const char *test_name, PyObject *op) { From 00e2c99916d7315f8e6bda7cb099c4d5baf7dae4 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 10 Dec 2024 13:48:01 +0000 Subject: [PATCH 5/7] Ensure the GIL is held for PyUnstable_AtExit --- Modules/atexitmodule.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Modules/atexitmodule.c b/Modules/atexitmodule.c index 6a4a5e7dd1e780..c009235b7a36c2 100644 --- a/Modules/atexitmodule.c +++ b/Modules/atexitmodule.c @@ -27,7 +27,10 @@ int PyUnstable_AtExit(PyInterpreterState *interp, atexit_datacallbackfunc func, void *data) { - assert(interp == _PyInterpreterState_GET()); + PyThreadState *tstate = _PyThreadState_GET(); + _Py_EnsureTstateNotNULL(tstate); + assert(tstate->interp == interp); + atexit_callback *callback = PyMem_Malloc(sizeof(atexit_callback)); if (callback == NULL) { PyErr_NoMemory(); From 71484c86fe17b852ecda15b5c37c3da544724599 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 10 Dec 2024 14:00:54 +0000 Subject: [PATCH 6/7] Document PyUnstable_AtExit --- Doc/c-api/init.rst | 9 +++++++++ Doc/c-api/sys.rst | 4 ++++ 2 files changed, 13 insertions(+) diff --git a/Doc/c-api/init.rst b/Doc/c-api/init.rst index ba1c2852f0bd53..dd63dd013e32dc 100644 --- a/Doc/c-api/init.rst +++ b/Doc/c-api/init.rst @@ -567,6 +567,15 @@ Initializing and finalizing the interpreter customized Python that always runs in isolated mode using :c:func:`Py_RunMain`. +.. c:function:: int PyUnstable_AtExit(PyInterpreterState *interp, void (*func)(void *), void *data) + + Register an :mod:`atexit` callback for the target interpreter *interp*. + This is similar to :c:func:`Py_AtExit`, but takes an explicit interpreter and + data pointer for the callback. + + The :term:`GIL` must be held for *interp*. + + .. versionadded:: 3.13 Process-wide parameters ======================= diff --git a/Doc/c-api/sys.rst b/Doc/c-api/sys.rst index d6fca1a0b0a219..c688afdca8231d 100644 --- a/Doc/c-api/sys.rst +++ b/Doc/c-api/sys.rst @@ -426,3 +426,7 @@ Process Control function registered last is called first. Each cleanup function will be called at most once. Since Python's internal finalization will have completed before the cleanup function, no Python APIs should be called by *func*. + + .. seealso:: + + :c:func:`PyUnstable_AtExit` for passing a ``void *data`` argument. From 149dc96b773d2f0bbf973be072138fbbb87491c2 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 10 Dec 2024 14:25:29 +0000 Subject: [PATCH 7/7] Add blurb. --- .../next/C_API/2024-12-10-14-25-22.gh-issue-127791.YRw4GU.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/C_API/2024-12-10-14-25-22.gh-issue-127791.YRw4GU.rst diff --git a/Misc/NEWS.d/next/C_API/2024-12-10-14-25-22.gh-issue-127791.YRw4GU.rst b/Misc/NEWS.d/next/C_API/2024-12-10-14-25-22.gh-issue-127791.YRw4GU.rst new file mode 100644 index 00000000000000..70751f18f5cf17 --- /dev/null +++ b/Misc/NEWS.d/next/C_API/2024-12-10-14-25-22.gh-issue-127791.YRw4GU.rst @@ -0,0 +1,2 @@ +Fix loss of callbacks after more than one call to +:c:func:`PyUnstable_AtExit`.