bpo-42639: Cleanup atexitmodule.c (GH-23770)

* Rename "atexitmodule_state" to "struct atexit_state".
* Rename "modstate" to "state".
* Rename "self" parameter to "module".
* test_atexit uses textwrap.dedent().
* Remove _Py_PyAtExit() function: inline it into atexit_exec().
* PyInterpreterState: rename pyexitfunc to atexit_func, rename
  pyexitmodule to atexit_module.
This commit is contained in:
Victor Stinner 2020-12-14 22:40:40 +01:00 committed by GitHub
parent fdb9efce6a
commit 83d52044ae
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 126 additions and 159 deletions

View File

@ -35,11 +35,6 @@ PyAPI_FUNC(int) Py_RunMain(void);
PyAPI_FUNC(void) _Py_NO_RETURN Py_ExitStatusException(PyStatus err); PyAPI_FUNC(void) _Py_NO_RETURN Py_ExitStatusException(PyStatus err);
/* Py_PyAtExit is for the atexit module, Py_AtExit is for low-level
* exit functions.
*/
PyAPI_FUNC(void) _Py_PyAtExit(void (*func)(PyObject *), PyObject *);
/* Restore signals that the interpreter has called SIG_IGN on to SIG_DFL. */ /* Restore signals that the interpreter has called SIG_IGN on to SIG_DFL. */
PyAPI_FUNC(void) _Py_RestoreSignals(void); PyAPI_FUNC(void) _Py_RestoreSignals(void);

View File

@ -234,8 +234,8 @@ struct _is {
PyObject *after_forkers_child; PyObject *after_forkers_child;
#endif #endif
/* AtExit module */ /* AtExit module */
void (*pyexitfunc)(PyObject *); void (*atexit_func)(PyObject *);
PyObject *pyexitmodule; PyObject *atexit_module;
uint64_t tstate_next_unique_id; uint64_t tstate_next_unique_id;

View File

@ -1,8 +1,9 @@
import sys
import unittest
import io
import atexit import atexit
import io
import os import os
import sys
import textwrap
import unittest
from test import support from test import support
from test.support import script_helper from test.support import script_helper
@ -156,7 +157,7 @@ class GeneralTest(unittest.TestCase):
def test_shutdown(self): def test_shutdown(self):
# Actually test the shutdown mechanism in a subprocess # Actually test the shutdown mechanism in a subprocess
code = """if 1: code = textwrap.dedent("""
import atexit import atexit
def f(msg): def f(msg):
@ -164,7 +165,7 @@ class GeneralTest(unittest.TestCase):
atexit.register(f, "one") atexit.register(f, "one")
atexit.register(f, "two") atexit.register(f, "two")
""" """)
res = script_helper.assert_python_ok("-c", code) res = script_helper.assert_python_ok("-c", code)
self.assertEqual(res.out.decode().splitlines(), ["two", "one"]) self.assertEqual(res.out.decode().splitlines(), ["two", "one"])
self.assertFalse(res.err) self.assertFalse(res.err)
@ -178,13 +179,13 @@ class SubinterpreterTest(unittest.TestCase):
# take care to free callbacks in its per-subinterpreter module # take care to free callbacks in its per-subinterpreter module
# state. # state.
n = atexit._ncallbacks() n = atexit._ncallbacks()
code = r"""if 1: code = textwrap.dedent(r"""
import atexit import atexit
def f(): def f():
pass pass
atexit.register(f) atexit.register(f)
del atexit del atexit
""" """)
ret = support.run_in_subinterp(code) ret = support.run_in_subinterp(code)
self.assertEqual(ret, 0) self.assertEqual(ret, 0)
self.assertEqual(atexit._ncallbacks(), n) self.assertEqual(atexit._ncallbacks(), n)
@ -193,13 +194,13 @@ class SubinterpreterTest(unittest.TestCase):
# Similar to the above, but with a refcycle through the atexit # Similar to the above, but with a refcycle through the atexit
# module. # module.
n = atexit._ncallbacks() n = atexit._ncallbacks()
code = r"""if 1: code = textwrap.dedent(r"""
import atexit import atexit
def f(): def f():
pass pass
atexit.register(f) atexit.register(f)
atexit.__atexit = atexit atexit.__atexit = atexit
""" """)
ret = support.run_in_subinterp(code) ret = support.run_in_subinterp(code)
self.assertEqual(ret, 0) self.assertEqual(ret, 0)
self.assertEqual(atexit._ncallbacks(), n) self.assertEqual(atexit._ncallbacks(), n)
@ -210,13 +211,13 @@ class SubinterpreterTest(unittest.TestCase):
expected = b"The test has passed!" expected = b"The test has passed!"
r, w = os.pipe() r, w = os.pipe()
code = r"""if 1: code = textwrap.dedent(r"""
import os import os
import atexit import atexit
def callback(): def callback():
os.write({:d}, b"The test has passed!") os.write({:d}, b"The test has passed!")
atexit.register(callback) atexit.register(callback)
""".format(w) """.format(w))
ret = support.run_in_subinterp(code) ret = support.run_in_subinterp(code)
os.close(w) os.close(w)
self.assertEqual(os.read(r, len(expected)), expected) self.assertEqual(os.read(r, len(expected)), expected)

View File

@ -7,11 +7,8 @@
*/ */
#include "Python.h" #include "Python.h"
#include "pycore_interp.h" // PyInterpreterState.atexit_func
/* Forward declaration (for atexit_cleanup) */ #include "pycore_pystate.h" // _PyInterpreterState_GET
static PyObject *atexit_clear(PyObject*, PyObject*);
/* Forward declaration of module object */
static struct PyModuleDef atexitmodule;
/* ===================================================================== */ /* ===================================================================== */
/* Callback machinery. */ /* Callback machinery. */
@ -22,48 +19,47 @@ typedef struct {
PyObject *kwargs; PyObject *kwargs;
} atexit_callback; } atexit_callback;
typedef struct { struct atexit_state {
atexit_callback **atexit_callbacks; atexit_callback **atexit_callbacks;
int ncallbacks; int ncallbacks;
int callback_len; int callback_len;
} atexitmodule_state; };
static inline atexitmodule_state* static inline struct atexit_state*
get_atexit_state(PyObject *module) get_atexit_state(PyObject *module)
{ {
void *state = PyModule_GetState(module); void *state = PyModule_GetState(module);
assert(state != NULL); assert(state != NULL);
return (atexitmodule_state *)state; return (struct atexit_state *)state;
} }
static void static void
atexit_delete_cb(atexitmodule_state *modstate, int i) atexit_delete_cb(struct atexit_state *state, int i)
{ {
atexit_callback *cb; atexit_callback *cb = state->atexit_callbacks[i];
state->atexit_callbacks[i] = NULL;
cb = modstate->atexit_callbacks[i];
modstate->atexit_callbacks[i] = NULL;
Py_DECREF(cb->func); Py_DECREF(cb->func);
Py_DECREF(cb->args); Py_DECREF(cb->args);
Py_XDECREF(cb->kwargs); Py_XDECREF(cb->kwargs);
PyMem_Free(cb); PyMem_Free(cb);
} }
/* Clear all callbacks without calling them */ /* Clear all callbacks without calling them */
static void static void
atexit_cleanup(atexitmodule_state *modstate) atexit_cleanup(struct atexit_state *state)
{ {
atexit_callback *cb; atexit_callback *cb;
int i; for (int i = 0; i < state->ncallbacks; i++) {
for (i = 0; i < modstate->ncallbacks; i++) { cb = state->atexit_callbacks[i];
cb = modstate->atexit_callbacks[i];
if (cb == NULL) if (cb == NULL)
continue; continue;
atexit_delete_cb(modstate, i); atexit_delete_cb(state, i);
} }
modstate->ncallbacks = 0; state->ncallbacks = 0;
} }
/* Installed into pylifecycle.c's atexit mechanism */ /* Installed into pylifecycle.c's atexit mechanism */
@ -71,28 +67,26 @@ atexit_cleanup(atexitmodule_state *modstate)
static void static void
atexit_callfuncs(PyObject *module) atexit_callfuncs(PyObject *module)
{ {
PyObject *exc_type = NULL, *exc_value, *exc_tb, *r; assert(!PyErr_Occurred());
atexit_callback *cb;
atexitmodule_state *modstate;
int i;
if (module == NULL) if (module == NULL) {
return; return;
modstate = get_atexit_state(module); }
if (modstate->ncallbacks == 0) struct atexit_state *state = get_atexit_state(module);
if (state->ncallbacks == 0) {
return; return;
}
PyObject *exc_type = NULL, *exc_value, *exc_tb;
for (i = modstate->ncallbacks - 1; i >= 0; i--) for (int i = state->ncallbacks - 1; i >= 0; i--) {
{ atexit_callback *cb = state->atexit_callbacks[i];
cb = modstate->atexit_callbacks[i]; if (cb == NULL) {
if (cb == NULL)
continue; continue;
}
r = PyObject_Call(cb->func, cb->args, cb->kwargs); PyObject *res = PyObject_Call(cb->func, cb->args, cb->kwargs);
Py_XDECREF(r); if (res == NULL) {
if (r == NULL) {
/* Maintain the last exception, but don't leak if there are /* Maintain the last exception, but don't leak if there are
multiple exceptions. */ multiple exceptions. */
if (exc_type) { if (exc_type) {
@ -107,12 +101,16 @@ atexit_callfuncs(PyObject *module)
PyErr_Display(exc_type, exc_value, exc_tb); PyErr_Display(exc_type, exc_value, exc_tb);
} }
} }
else {
Py_DECREF(res);
}
} }
atexit_cleanup(modstate); atexit_cleanup(state);
if (exc_type) if (exc_type) {
PyErr_Restore(exc_type, exc_value, exc_tb); PyErr_Restore(exc_type, exc_value, exc_tb);
}
} }
/* ===================================================================== */ /* ===================================================================== */
@ -130,55 +128,48 @@ Register a function to be executed upon normal program termination\n\
func is returned to facilitate usage as a decorator."); func is returned to facilitate usage as a decorator.");
static PyObject * static PyObject *
atexit_register(PyObject *self, PyObject *args, PyObject *kwargs) atexit_register(PyObject *module, PyObject *args, PyObject *kwargs)
{ {
atexitmodule_state *modstate;
atexit_callback *new_callback;
PyObject *func = NULL;
modstate = get_atexit_state(self);
if (modstate->ncallbacks >= modstate->callback_len) {
atexit_callback **r;
modstate->callback_len += 16;
r = (atexit_callback**)PyMem_Realloc(modstate->atexit_callbacks,
sizeof(atexit_callback*) * modstate->callback_len);
if (r == NULL)
return PyErr_NoMemory();
modstate->atexit_callbacks = r;
}
if (PyTuple_GET_SIZE(args) == 0) { if (PyTuple_GET_SIZE(args) == 0) {
PyErr_SetString(PyExc_TypeError, PyErr_SetString(PyExc_TypeError,
"register() takes at least 1 argument (0 given)"); "register() takes at least 1 argument (0 given)");
return NULL; return NULL;
} }
func = PyTuple_GET_ITEM(args, 0); PyObject *func = PyTuple_GET_ITEM(args, 0);
if (!PyCallable_Check(func)) { if (!PyCallable_Check(func)) {
PyErr_SetString(PyExc_TypeError, PyErr_SetString(PyExc_TypeError,
"the first argument must be callable"); "the first argument must be callable");
return NULL; return NULL;
} }
new_callback = PyMem_Malloc(sizeof(atexit_callback)); struct atexit_state *state = get_atexit_state(module);
if (new_callback == NULL) if (state->ncallbacks >= state->callback_len) {
return PyErr_NoMemory(); atexit_callback **r;
state->callback_len += 16;
r = (atexit_callback**)PyMem_Realloc(state->atexit_callbacks,
sizeof(atexit_callback*) * state->callback_len);
if (r == NULL)
return PyErr_NoMemory();
state->atexit_callbacks = r;
}
new_callback->args = PyTuple_GetSlice(args, 1, PyTuple_GET_SIZE(args)); atexit_callback *callback = PyMem_Malloc(sizeof(atexit_callback));
if (new_callback->args == NULL) { if (callback == NULL) {
PyMem_Free(new_callback); return PyErr_NoMemory();
}
callback->args = PyTuple_GetSlice(args, 1, PyTuple_GET_SIZE(args));
if (callback->args == NULL) {
PyMem_Free(callback);
return NULL; return NULL;
} }
new_callback->func = func; callback->func = Py_NewRef(func);
new_callback->kwargs = kwargs; callback->kwargs = Py_XNewRef(kwargs);
Py_INCREF(func);
Py_XINCREF(kwargs);
modstate->atexit_callbacks[modstate->ncallbacks++] = new_callback; state->atexit_callbacks[state->ncallbacks++] = callback;
Py_INCREF(func); return Py_NewRef(func);
return func;
} }
PyDoc_STRVAR(atexit_run_exitfuncs__doc__, PyDoc_STRVAR(atexit_run_exitfuncs__doc__,
@ -187,11 +178,12 @@ PyDoc_STRVAR(atexit_run_exitfuncs__doc__,
Run all registered exit functions."); Run all registered exit functions.");
static PyObject * static PyObject *
atexit_run_exitfuncs(PyObject *self, PyObject *unused) atexit_run_exitfuncs(PyObject *module, PyObject *unused)
{ {
atexit_callfuncs(self); atexit_callfuncs(module);
if (PyErr_Occurred()) if (PyErr_Occurred()) {
return NULL; return NULL;
}
Py_RETURN_NONE; Py_RETURN_NONE;
} }
@ -201,9 +193,9 @@ PyDoc_STRVAR(atexit_clear__doc__,
Clear the list of previously registered exit functions."); Clear the list of previously registered exit functions.");
static PyObject * static PyObject *
atexit_clear(PyObject *self, PyObject *unused) atexit_clear(PyObject *module, PyObject *unused)
{ {
atexit_cleanup(get_atexit_state(self)); atexit_cleanup(get_atexit_state(module));
Py_RETURN_NONE; Py_RETURN_NONE;
} }
@ -213,25 +205,18 @@ PyDoc_STRVAR(atexit_ncallbacks__doc__,
Return the number of registered exit functions."); Return the number of registered exit functions.");
static PyObject * static PyObject *
atexit_ncallbacks(PyObject *self, PyObject *unused) atexit_ncallbacks(PyObject *module, PyObject *unused)
{ {
atexitmodule_state *modstate; struct atexit_state *state = get_atexit_state(module);
return PyLong_FromSsize_t(state->ncallbacks);
modstate = get_atexit_state(self);
return PyLong_FromSsize_t(modstate->ncallbacks);
} }
static int static int
atexit_m_traverse(PyObject *self, visitproc visit, void *arg) atexit_m_traverse(PyObject *module, visitproc visit, void *arg)
{ {
int i; struct atexit_state *state = (struct atexit_state *)PyModule_GetState(module);
atexitmodule_state *modstate; for (int i = 0; i < state->ncallbacks; i++) {
atexit_callback *cb = state->atexit_callbacks[i];
modstate = (atexitmodule_state *)PyModule_GetState(self);
for (i = 0; i < modstate->ncallbacks; i++) {
atexit_callback *cb = modstate->atexit_callbacks[i];
if (cb == NULL) if (cb == NULL)
continue; continue;
Py_VISIT(cb->func); Py_VISIT(cb->func);
@ -242,21 +227,19 @@ atexit_m_traverse(PyObject *self, visitproc visit, void *arg)
} }
static int static int
atexit_m_clear(PyObject *self) atexit_m_clear(PyObject *module)
{ {
atexitmodule_state *modstate; struct atexit_state *state = (struct atexit_state *)PyModule_GetState(module);
modstate = (atexitmodule_state *)PyModule_GetState(self); atexit_cleanup(state);
atexit_cleanup(modstate);
return 0; return 0;
} }
static void static void
atexit_free(PyObject *m) atexit_free(PyObject *module)
{ {
atexitmodule_state *modstate; struct atexit_state *state = (struct atexit_state *)PyModule_GetState(module);
modstate = (atexitmodule_state *)PyModule_GetState(m); atexit_cleanup(state);
atexit_cleanup(modstate); PyMem_Free(state->atexit_callbacks);
PyMem_Free(modstate->atexit_callbacks);
} }
PyDoc_STRVAR(atexit_unregister__doc__, PyDoc_STRVAR(atexit_unregister__doc__,
@ -268,29 +251,28 @@ atexit.register\n\
func - function to be unregistered"); func - function to be unregistered");
static PyObject * static PyObject *
atexit_unregister(PyObject *self, PyObject *func) atexit_unregister(PyObject *module, PyObject *func)
{ {
atexitmodule_state *modstate; struct atexit_state *state = get_atexit_state(module);
atexit_callback *cb; for (int i = 0; i < state->ncallbacks; i++)
int i, eq;
modstate = get_atexit_state(self);
for (i = 0; i < modstate->ncallbacks; i++)
{ {
cb = modstate->atexit_callbacks[i]; atexit_callback *cb = state->atexit_callbacks[i];
if (cb == NULL) if (cb == NULL) {
continue; continue;
}
eq = PyObject_RichCompareBool(cb->func, func, Py_EQ); int eq = PyObject_RichCompareBool(cb->func, func, Py_EQ);
if (eq < 0) if (eq < 0) {
return NULL; return NULL;
if (eq) }
atexit_delete_cb(modstate, i); if (eq) {
atexit_delete_cb(state, i);
}
} }
Py_RETURN_NONE; Py_RETURN_NONE;
} }
static PyMethodDef atexit_methods[] = { static PyMethodDef atexit_methods[] = {
{"register", (PyCFunction)(void(*)(void)) atexit_register, METH_VARARGS|METH_KEYWORDS, {"register", (PyCFunction)(void(*)(void)) atexit_register, METH_VARARGS|METH_KEYWORDS,
atexit_register__doc__}, atexit_register__doc__},
@ -316,18 +298,19 @@ Two public functions, register and unregister, are defined.\n\
"); ");
static int static int
atexit_exec(PyObject *m) { atexit_exec(PyObject *module)
atexitmodule_state *modstate; {
struct atexit_state *state = get_atexit_state(module);
modstate = get_atexit_state(m); state->callback_len = 32;
modstate->callback_len = 32; state->ncallbacks = 0;
modstate->ncallbacks = 0; state->atexit_callbacks = PyMem_New(atexit_callback*, state->callback_len);
modstate->atexit_callbacks = PyMem_New(atexit_callback*, if (state->atexit_callbacks == NULL) {
modstate->callback_len);
if (modstate->atexit_callbacks == NULL)
return -1; return -1;
}
_Py_PyAtExit(atexit_callfuncs, m); PyInterpreterState *is = _PyInterpreterState_GET();
is->atexit_func = atexit_callfuncs;
is->atexit_module = module;
return 0; return 0;
} }
@ -338,14 +321,14 @@ static PyModuleDef_Slot atexit_slots[] = {
static struct PyModuleDef atexitmodule = { static struct PyModuleDef atexitmodule = {
PyModuleDef_HEAD_INIT, PyModuleDef_HEAD_INIT,
"atexit", .m_name = "atexit",
atexit__doc__, .m_doc = atexit__doc__,
sizeof(atexitmodule_state), .m_size = sizeof(struct atexit_state),
atexit_methods, .m_methods = atexit_methods,
atexit_slots, .m_slots = atexit_slots,
atexit_m_traverse, .m_traverse = atexit_m_traverse,
atexit_m_clear, .m_clear = atexit_m_clear,
(freefunc)atexit_free .m_free = (freefunc)atexit_free
}; };
PyMODINIT_FUNC PyMODINIT_FUNC

View File

@ -2634,26 +2634,14 @@ Py_ExitStatusException(PyStatus status)
/* Clean up and exit */ /* Clean up and exit */
/* For the atexit module. */
void _Py_PyAtExit(void (*func)(PyObject *), PyObject *module)
{
PyInterpreterState *is = _PyInterpreterState_GET();
/* Guard against API misuse (see bpo-17852) */
assert(is->pyexitfunc == NULL || is->pyexitfunc == func);
is->pyexitfunc = func;
is->pyexitmodule = module;
}
static void static void
call_py_exitfuncs(PyThreadState *tstate) call_py_exitfuncs(PyThreadState *tstate)
{ {
PyInterpreterState *interp = tstate->interp; PyInterpreterState *interp = tstate->interp;
if (interp->pyexitfunc == NULL) if (interp->atexit_func == NULL)
return; return;
(*interp->pyexitfunc)(interp->pyexitmodule); interp->atexit_func(interp->atexit_module);
_PyErr_Clear(tstate); _PyErr_Clear(tstate);
} }