Skip to content

gh-127791: Fix, document, and test PyUnstable_AtExit #127793

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Doc/c-api/init.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
=======================
Expand Down
4 changes: 4 additions & 0 deletions Doc/c-api/sys.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
1 change: 0 additions & 1 deletion Include/internal/pycore_atexit.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix loss of callbacks after more than one call to
:c:func:`PyUnstable_AtExit`.
48 changes: 48 additions & 0 deletions Modules/_testcapimodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -3353,6 +3353,53 @@ 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))
{
PyThreadState *oldts = PyThreadState_Swap(NULL);
PyThreadState *tstate = Py_NewInterpreter();

struct atexit_data data = {0};
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 (data.called != amount) {
PyErr_SetString(PyExc_RuntimeError, "atexit callback not called");
return NULL;
}
Py_RETURN_NONE;
}

static PyMethodDef TestMethods[] = {
{"set_errno", set_errno, METH_VARARGS},
Expand Down Expand Up @@ -3495,6 +3542,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 */
};

Expand Down
34 changes: 0 additions & 34 deletions Modules/_testinternalcapi.c
Original file line number Diff line number Diff line change
Expand Up @@ -1236,39 +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_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)
{
Expand Down Expand Up @@ -2128,7 +2095,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},
Expand Down
12 changes: 8 additions & 4 deletions Modules/atexitmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Comment on lines +30 to +31
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use PyInterpreterState_Get(), or PyThreadState_Get(), instead: it does the same.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to be explicit rather than do it with one of those.

assert(tstate->interp == interp);

atexit_callback *callback = PyMem_Malloc(sizeof(atexit_callback));
if (callback == NULL) {
PyErr_NoMemory();
Expand All @@ -38,12 +41,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;
}
Expand Down
Loading