bpo-42639: atexit now logs callbacks exceptions (GH-23771)

At Python exit, if a callback registered with atexit.register()
fails, its exception is now logged. Previously, only some exceptions
were logged, and the last exception was always silently ignored.

Add _PyAtExit_Call() function and remove
PyInterpreterState.atexit_func member. call_py_exitfuncs() now calls
directly _PyAtExit_Call().

The atexit module must now always be built as a built-in module.

Doc/whatsnew/3.10.rst

@@ -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
 ========================
 
@@ -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

Include/internal/pycore_interp.h

@@ -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;

Include/internal/pycore_pylifecycle.h

@@ -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

Lib/test/test_threading.py

@@ -487,7 +487,6 @@ class ThreadTests(BaseTestCase):
                 if not pid:
                     print("child process ok", file=sys.stderr, flush=True)
                     # child process
-                    sys.exit()
                 else:
                     wait_process(pid, exitcode=0)
 

Misc/NEWS.d/next/Library/2020-12-14-22-31-22.bpo-42639.5Z3iWX.rst

@@ -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.

Modules/atexitmodule.c

@@ -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());
 
@@ -87,6 +87,10 @@ atexit_callfuncs(PyObject *module)
 
         PyObject *res = PyObject_Call(cb->func, cb->args, cb->kwargs);
         if (res == NULL) {
+            if (ignore_exc) {
+                _PyErr_WriteUnraisableMsg("in atexit callback", cb->func);
+            }
+            else {
                 /* Maintain the last exception, but don't leak if there are
                    multiple exceptions. */
                 if (exc_type) {
@@ -101,6 +105,7 @@ atexit_callfuncs(PyObject *module)
                     PyErr_Display(exc_type, exc_value, exc_tb);
                 }
             }
+        }
         else {
             Py_DECREF(res);
         }
@@ -108,10 +113,23 @@ atexit_callfuncs(PyObject *module)
 
     atexit_cleanup(state);
 
+    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. */
@@ -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;
     }
@@ -308,9 +326,8 @@ atexit_exec(PyObject *module)
         return -1;
     }
 
-    PyInterpreterState *is = _PyInterpreterState_GET();
+    PyInterpreterState *interp = _PyInterpreterState_GET();
-    is->atexit_func = atexit_callfuncs;
+    interp->atexit_module = module;
-    is->atexit_module = module;
     return 0;
 }
 

Python/pylifecycle.c

@@ -2632,19 +2632,16 @@ Py_ExitStatusException(PyStatus status)
     }
 }
 
+
 /* Clean up and exit */
 
 static void
 call_py_exitfuncs(PyThreadState *tstate)
 {
-    PyInterpreterState *interp = tstate->interp;
+    _PyAtExit_Call(tstate->interp->atexit_module);
-    if (interp->atexit_func == NULL)
-        return;
-
-    interp->atexit_func(interp->atexit_module);
-    _PyErr_Clear(tstate);
 }
 
+
 /* Wait until threading._shutdown completes, provided
    the threading module was imported in the first place.
    The shutdown routine will wait until all non-daemon

setup.py

@@ -854,8 +854,6 @@ class PyBuildExt(build_ext):
         # 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']))