Skip to content

bpo-42639: atexit now logs callbacks exceptions #23771

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 1 commit into from
Dec 14, 2020
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/whatsnew/3.10.rst
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,13 @@ Changes in the Python API
have been renamed to *exc*.
(Contributed by Zackery Spytz and Matthias Bussonnier in :issue:`26389`.)

* :mod:`atexit`: At Python exit, if a callback registered with
:func:`atexit.register` fails, its exception is now logged. Previously, only
some exceptions were logged, and the last exception was always silently
ignored.
(Contributed by Victor Stinner in :issue:`42639`.)


CPython bytecode changes
========================

Expand All @@ -519,6 +526,8 @@ Build Changes
* :mod:`sqlite3` requires SQLite 3.7.3 or higher.
(Contributed by Sergey Fedoseev and Erlend E. Aasland :issue:`40744`.)

* The :mod:`atexit` module must now always be built as a built-in module.
(Contributed by Victor Stinner in :issue:`42639`.)


C API Changes
Expand Down
2 changes: 1 addition & 1 deletion Include/internal/pycore_interp.h
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,8 @@ struct _is {
PyObject *after_forkers_parent;
PyObject *after_forkers_child;
#endif

/* AtExit module */
void (*atexit_func)(PyObject *);
PyObject *atexit_module;

uint64_t tstate_next_unique_id;
Expand Down
2 changes: 2 additions & 0 deletions Include/internal/pycore_pylifecycle.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ PyAPI_FUNC(void) _PyErr_Display(PyObject *file, PyObject *exception,

PyAPI_FUNC(void) _PyThreadState_DeleteCurrent(PyThreadState *tstate);

extern void _PyAtExit_Call(PyObject *module);

#ifdef __cplusplus
}
#endif
Expand Down
1 change: 0 additions & 1 deletion Lib/test/test_threading.py
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,6 @@ def exit_handler():
if not pid:
print("child process ok", file=sys.stderr, flush=True)
# child process
sys.exit()
else:
wait_process(pid, exitcode=0)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
At Python exit, if a callback registered with :func:`atexit.register` fails,
its exception is now logged. Previously, only some exceptions were logged, and
the last exception was always silently ignored.
53 changes: 35 additions & 18 deletions Modules/atexitmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ atexit_cleanup(struct atexit_state *state)
/* Installed into pylifecycle.c's atexit mechanism */

static void
atexit_callfuncs(PyObject *module)
atexit_callfuncs(PyObject *module, int ignore_exc)
{
assert(!PyErr_Occurred());

Expand All @@ -87,18 +87,23 @@ atexit_callfuncs(PyObject *module)

PyObject *res = PyObject_Call(cb->func, cb->args, cb->kwargs);
if (res == NULL) {
/* Maintain the last exception, but don't leak if there are
multiple exceptions. */
if (exc_type) {
Py_DECREF(exc_type);
Py_XDECREF(exc_value);
Py_XDECREF(exc_tb);
if (ignore_exc) {
_PyErr_WriteUnraisableMsg("in atexit callback", cb->func);
}
PyErr_Fetch(&exc_type, &exc_value, &exc_tb);
if (!PyErr_GivenExceptionMatches(exc_type, PyExc_SystemExit)) {
PySys_WriteStderr("Error in atexit._run_exitfuncs:\n");
PyErr_NormalizeException(&exc_type, &exc_value, &exc_tb);
PyErr_Display(exc_type, exc_value, exc_tb);
else {
/* Maintain the last exception, but don't leak if there are
multiple exceptions. */
if (exc_type) {
Py_DECREF(exc_type);
Py_XDECREF(exc_value);
Py_XDECREF(exc_tb);
}
PyErr_Fetch(&exc_type, &exc_value, &exc_tb);
if (!PyErr_GivenExceptionMatches(exc_type, PyExc_SystemExit)) {
PySys_WriteStderr("Error in atexit._run_exitfuncs:\n");
PyErr_NormalizeException(&exc_type, &exc_value, &exc_tb);
PyErr_Display(exc_type, exc_value, exc_tb);
}
}
}
else {
Expand All @@ -108,11 +113,24 @@ atexit_callfuncs(PyObject *module)

atexit_cleanup(state);

if (exc_type) {
PyErr_Restore(exc_type, exc_value, exc_tb);
if (ignore_exc) {
assert(!PyErr_Occurred());
}
else {
if (exc_type) {
PyErr_Restore(exc_type, exc_value, exc_tb);
}
}
}


void
_PyAtExit_Call(PyObject *module)
{
atexit_callfuncs(module, 1);
}


/* ===================================================================== */
/* Module methods. */

Expand Down Expand Up @@ -180,7 +198,7 @@ Run all registered exit functions.");
static PyObject *
atexit_run_exitfuncs(PyObject *module, PyObject *unused)
{
atexit_callfuncs(module);
atexit_callfuncs(module, 0);
if (PyErr_Occurred()) {
return NULL;
}
Expand Down Expand Up @@ -308,9 +326,8 @@ atexit_exec(PyObject *module)
return -1;
}

PyInterpreterState *is = _PyInterpreterState_GET();
is->atexit_func = atexit_callfuncs;
is->atexit_module = module;
PyInterpreterState *interp = _PyInterpreterState_GET();
interp->atexit_module = module;
return 0;
}

Expand Down
9 changes: 3 additions & 6 deletions Python/pylifecycle.c
Original file line number Diff line number Diff line change
Expand Up @@ -2632,19 +2632,16 @@ Py_ExitStatusException(PyStatus status)
}
}


/* Clean up and exit */

static void
call_py_exitfuncs(PyThreadState *tstate)
{
PyInterpreterState *interp = tstate->interp;
if (interp->atexit_func == NULL)
return;

interp->atexit_func(interp->atexit_module);
_PyErr_Clear(tstate);
_PyAtExit_Call(tstate->interp->atexit_module);
}


/* Wait until threading._shutdown completes, provided
the threading module was imported in the first place.
The shutdown routine will wait until all non-daemon
Expand Down
2 changes: 0 additions & 2 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -854,8 +854,6 @@ def detect_simple_extensions(self):
# C-optimized pickle replacement
self.add(Extension("_pickle", ["_pickle.c"],
extra_compile_args=['-DPy_BUILD_CORE_MODULE']))
# atexit
self.add(Extension("atexit", ["atexitmodule.c"]))
# _json speedups
self.add(Extension("_json", ["_json.c"],
extra_compile_args=['-DPy_BUILD_CORE_MODULE']))
Expand Down