bpo-42639: atexit._run_exitfuncs() uses sys.unraisablehook (GH-23779)

atexit._run_exitfuncs() now logs callback exceptions using
sys.unraisablehook, rather than logging them directly into
sys.stderr and raising the last exception.

Run GeneralTest of test_atexit in a subprocess since it calls
atexit._clear() which clears all atexit callbacks.

_PyAtExit_Fini() sets state->callbacks to NULL.

Lib/test/_test_atexit.py

@@ -0,0 +1,121 @@
+"""
+Tests run by test_atexit in a subprocess since it clears atexit callbacks.
+"""
+import atexit
+import sys
+import unittest
+from test import support
+
+
+class GeneralTest(unittest.TestCase):
+    def setUp(self):
+        atexit._clear()
+
+    def tearDown(self):
+        atexit._clear()
+
+    def assert_raises_unraisable(self, exc_type, func, *args):
+        with support.catch_unraisable_exception() as cm:
+            atexit.register(func, *args)
+            atexit._run_exitfuncs()
+
+            self.assertEqual(cm.unraisable.object, func)
+            self.assertEqual(cm.unraisable.exc_type, exc_type)
+            self.assertEqual(type(cm.unraisable.exc_value), exc_type)
+
+    def test_order(self):
+        # Check that callbacks are called in reverse order with the expected
+        # positional and keyword arguments.
+        calls = []
+
+        def func1(*args, **kwargs):
+            calls.append(('func1', args, kwargs))
+
+        def func2(*args, **kwargs):
+            calls.append(('func2', args, kwargs))
+
+        # be sure args are handled properly
+        atexit.register(func1, 1, 2)
+        atexit.register(func2)
+        atexit.register(func2, 3, key="value")
+        atexit._run_exitfuncs()
+
+        self.assertEqual(calls,
+                         [('func2', (3,), {'key': 'value'}),
+                          ('func2', (), {}),
+                          ('func1', (1, 2), {})])
+
+    def test_badargs(self):
+        def func():
+            pass
+
+        # func() has no parameter, but it's called with 2 parameters
+        self.assert_raises_unraisable(TypeError, func, 1 ,2)
+
+    def test_raise(self):
+        def raise_type_error():
+            raise TypeError
+
+        self.assert_raises_unraisable(TypeError, raise_type_error)
+
+    def test_raise_unnormalized(self):
+        # bpo-10756: Make sure that an unnormalized exception is handled
+        # properly.
+        def div_zero():
+            1 / 0
+
+        self.assert_raises_unraisable(ZeroDivisionError, div_zero)
+
+    def test_exit(self):
+        self.assert_raises_unraisable(SystemExit, sys.exit)
+
+    def test_stress(self):
+        a = [0]
+        def inc():
+            a[0] += 1
+
+        for i in range(128):
+            atexit.register(inc)
+        atexit._run_exitfuncs()
+
+        self.assertEqual(a[0], 128)
+
+    def test_clear(self):
+        a = [0]
+        def inc():
+            a[0] += 1
+
+        atexit.register(inc)
+        atexit._clear()
+        atexit._run_exitfuncs()
+
+        self.assertEqual(a[0], 0)
+
+    def test_unregister(self):
+        a = [0]
+        def inc():
+            a[0] += 1
+        def dec():
+            a[0] -= 1
+
+        for i in range(4):
+            atexit.register(inc)
+        atexit.register(dec)
+        atexit.unregister(inc)
+        atexit._run_exitfuncs()
+
+        self.assertEqual(a[0], -1)
+
+    def test_bound_methods(self):
+        l = []
+        atexit.register(l.append, 5)
+        atexit._run_exitfuncs()
+        self.assertEqual(l, [5])
+
+        atexit.unregister(l.append)
+        atexit._run_exitfuncs()
+        self.assertEqual(l, [5])
+
+
+if __name__ == "__main__":
+    unittest.main()

Lib/test/test_atexit.py

@@ -1,5 +1,4 @@
 import atexit
-import io
 import os
 import sys
 import textwrap
@@ -7,154 +6,14 @@
 from test import support
 from test.support import script_helper
 
-### helpers
-def h1():
-    print("h1")
-
-def h2():
-    print("h2")
-
-def h3():
-    print("h3")
-
-def h4(*args, **kwargs):
-    print("h4", args, kwargs)
-
-def raise1():
-    raise TypeError
-
-def raise2():
-    raise SystemError
-
-def exit():
-    raise SystemExit
-
 
 class GeneralTest(unittest.TestCase):
+    def test_general(self):
+        # Run _test_atexit.py in a subprocess since it calls atexit._clear()
+        script = support.findfile("_test_atexit.py")
+        script_helper.run_test_script(script)
 
-    def setUp(self):
-        self.save_stdout = sys.stdout
-        self.save_stderr = sys.stderr
-        self.stream = io.StringIO()
-        sys.stdout = sys.stderr = self.stream
-        atexit._clear()
-
-    def tearDown(self):
-        sys.stdout = self.save_stdout
-        sys.stderr = self.save_stderr
-        atexit._clear()
-
-    def test_args(self):
-        # be sure args are handled properly
-        atexit.register(h1)
-        atexit.register(h4)
-        atexit.register(h4, 4, kw="abc")
-        atexit._run_exitfuncs()
-
-        self.assertEqual(self.stream.getvalue(),
-                            "h4 (4,) {'kw': 'abc'}\nh4 () {}\nh1\n")
-
-    def test_badargs(self):
-        atexit.register(lambda: 1, 0, 0, (x for x in (1,2)), 0, 0)
-        self.assertRaises(TypeError, atexit._run_exitfuncs)
-
-    def test_order(self):
-        # be sure handlers are executed in reverse order
-        atexit.register(h1)
-        atexit.register(h2)
-        atexit.register(h3)
-        atexit._run_exitfuncs()
-
-        self.assertEqual(self.stream.getvalue(), "h3\nh2\nh1\n")
-
-    def test_raise(self):
-        # be sure raises are handled properly
-        atexit.register(raise1)
-        atexit.register(raise2)
-
-        self.assertRaises(TypeError, atexit._run_exitfuncs)
-
-    def test_raise_unnormalized(self):
-        # Issue #10756: Make sure that an unnormalized exception is
-        # handled properly
-        atexit.register(lambda: 1 / 0)
-
-        self.assertRaises(ZeroDivisionError, atexit._run_exitfuncs)
-        self.assertIn("ZeroDivisionError", self.stream.getvalue())
-
-    def test_exit(self):
-        # be sure a SystemExit is handled properly
-        atexit.register(exit)
-
-        self.assertRaises(SystemExit, atexit._run_exitfuncs)
-        self.assertEqual(self.stream.getvalue(), '')
-
-    def test_print_tracebacks(self):
-        # Issue #18776: the tracebacks should be printed when errors occur.
-        def f():
-            1/0  # one
-        def g():
-            1/0  # two
-        def h():
-            1/0  # three
-        atexit.register(f)
-        atexit.register(g)
-        atexit.register(h)
-
-        self.assertRaises(ZeroDivisionError, atexit._run_exitfuncs)
-        stderr = self.stream.getvalue()
-        self.assertEqual(stderr.count("ZeroDivisionError"), 3)
-        self.assertIn("# one", stderr)
-        self.assertIn("# two", stderr)
-        self.assertIn("# three", stderr)
-
-    def test_stress(self):
-        a = [0]
-        def inc():
-            a[0] += 1
-
-        for i in range(128):
-            atexit.register(inc)
-        atexit._run_exitfuncs()
-
-        self.assertEqual(a[0], 128)
-
-    def test_clear(self):
-        a = [0]
-        def inc():
-            a[0] += 1
-
-        atexit.register(inc)
-        atexit._clear()
-        atexit._run_exitfuncs()
-
-        self.assertEqual(a[0], 0)
-
-    def test_unregister(self):
-        a = [0]
-        def inc():
-            a[0] += 1
-        def dec():
-            a[0] -= 1
-
-        for i in range(4):
-            atexit.register(inc)
-        atexit.register(dec)
-        atexit.unregister(inc)
-        atexit._run_exitfuncs()
-
-        self.assertEqual(a[0], -1)
-
-    def test_bound_methods(self):
-        l = []
-        atexit.register(l.append, 5)
-        atexit._run_exitfuncs()
-        self.assertEqual(l, [5])
-
-        atexit.unregister(l.append)
-        atexit._run_exitfuncs()
-        self.assertEqual(l, [5])
-
+class FunctionalTest(unittest.TestCase):
     def test_shutdown(self):
         # Actually test the shutdown mechanism in a subprocess
         code = textwrap.dedent("""

Lib/test/test_eintr.py

@@ -1,9 +1,6 @@
 import os
 import signal
-import subprocess
-import sys
 import unittest
-
 from test import support
 from test.support import script_helper
 

Misc/NEWS.d/next/Library/2020-12-15-15-14-29.bpo-42639.uJ3h8I.rst

@@ -0,0 +1,3 @@
+:func:`atexit._run_exitfuncs` now logs callback exceptions using
+:data:`sys.unraisablehook`, rather than logging them directly into
+:data:`sys.stderr` and raise the last exception.

Modules/atexitmodule.c

@@ -74,11 +74,12 @@ _PyAtExit_Fini(PyInterpreterState *interp)
     struct atexit_state *state = &interp->atexit;
     atexit_cleanup(state);
     PyMem_Free(state->callbacks);
+    state->callbacks = NULL;
 }
 
 
 static void
-atexit_callfuncs(struct atexit_state *state, int ignore_exc)
+atexit_callfuncs(struct atexit_state *state)
 {
     assert(!PyErr_Occurred());
 
@@ -86,7 +87,6 @@ atexit_callfuncs(struct atexit_state *state, int ignore_exc)
         return;
     }
 
-    PyObject *exc_type = NULL, *exc_value, *exc_tb;
     for (int i = state->ncallbacks - 1; i >= 0; i--) {
         atexit_callback *cb = state->callbacks[i];
         if (cb == NULL) {
@@ -95,24 +95,7 @@ atexit_callfuncs(struct atexit_state *state, int ignore_exc)
 
         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) {
-                    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);
-                }
-            }
+            _PyErr_WriteUnraisableMsg("in atexit callback", cb->func);
         }
         else {
             Py_DECREF(res);
@@ -121,14 +104,7 @@ atexit_callfuncs(struct atexit_state *state, int ignore_exc)
 
     atexit_cleanup(state);
 
-    if (ignore_exc) {
-        assert(!PyErr_Occurred());
-    }
-    else {
-        if (exc_type) {
-            PyErr_Restore(exc_type, exc_value, exc_tb);
-        }
-    }
+    assert(!PyErr_Occurred());
 }
 
 
@@ -136,7 +112,7 @@ void
 _PyAtExit_Call(PyThreadState *tstate)
 {
     struct atexit_state *state = &tstate->interp->atexit;
-    atexit_callfuncs(state, 1);
+    atexit_callfuncs(state);
 }
 
 
@@ -177,8 +153,9 @@ atexit_register(PyObject *module, PyObject *args, PyObject *kwargs)
         state->callback_len += 16;
         size_t size = sizeof(atexit_callback*) * (size_t)state->callback_len;
         r = (atexit_callback**)PyMem_Realloc(state->callbacks, size);
-        if (r == NULL)
+        if (r == NULL) {
             return PyErr_NoMemory();
+        }
         state->callbacks = r;
     }
 
@@ -203,16 +180,15 @@ atexit_register(PyObject *module, PyObject *args, PyObject *kwargs)
 PyDoc_STRVAR(atexit_run_exitfuncs__doc__,
 "_run_exitfuncs() -> None\n\
 \n\
-Run all registered exit functions.");
+Run all registered exit functions.\n\
+\n\
+If a callaback raises an exception, it is logged with sys.unraisablehook.");
 
 static PyObject *
 atexit_run_exitfuncs(PyObject *module, PyObject *unused)
 {
     struct atexit_state *state = get_atexit_state();
-    atexit_callfuncs(state, 0);
-    if (PyErr_Occurred()) {
-        return NULL;
-    }
+    atexit_callfuncs(state);
     Py_RETURN_NONE;
 }