bpo-41162: Clear audit hooks later during finalization (GH-21222)

Author: zkonge

Lib/test/audit-tests.py

@@ -44,28 +44,6 @@ def __call__(self, event, args):
             raise self.exc_type("saw event " + event)
 
 
-class TestFinalizeHook:
-    """Used in the test_finalize_hooks function to ensure that hooks
-    are correctly cleaned up, that they are notified about the cleanup,
-    and are unable to prevent it.
-    """
-
-    def __init__(self):
-        print("Created", id(self), file=sys.stdout, flush=True)
-
-    def __call__(self, event, args):
-        # Avoid recursion when we call id() below
-        if event == "builtins.id":
-            return
-
-        print(event, id(self), file=sys.stdout, flush=True)
-
-        if event == "cpython._PySys_ClearAuditHooks":
-            raise RuntimeError("Should be ignored")
-        elif event == "cpython.PyInterpreterState_Clear":
-            raise RuntimeError("Should be ignored")
-
-
 # Simple helpers, since we are not in unittest here
 def assertEqual(x, y):
     if x != y:
@@ -128,10 +106,6 @@ def test_block_add_hook_baseexception():
                 pass
 
 
-def test_finalize_hooks():
-    sys.addaudithook(TestFinalizeHook())
-
-
 def test_pickle():
     import pickle
 

Lib/test/test_audit.py

@@ -51,22 +51,6 @@ def test_block_add_hook(self):
     def test_block_add_hook_baseexception(self):
         self.do_test("test_block_add_hook_baseexception")
 
-    def test_finalize_hooks(self):
-        returncode, events, stderr = self.run_python("test_finalize_hooks")
-        if stderr:
-            print(stderr, file=sys.stderr)
-        if returncode:
-            self.fail(stderr)
-
-        firstId = events[0][2]
-        self.assertSequenceEqual(
-            [
-                ("Created", " ", firstId),
-                ("cpython._PySys_ClearAuditHooks", " ", firstId),
-            ],
-            events,
-        )
-
     def test_pickle(self):
         support.import_module("pickle")
 

Misc/NEWS.d/next/Security/2020-07-03-20-41-29.bpo-41162.tb8pVj.rst

@@ -0,0 +1 @@
+Audit hooks are now cleared later during finalization to avoid missing events.

Programs/_testembed.c

@@ -1112,8 +1112,11 @@ static int test_open_code_hook(void)
     return result;
 }
 
+static int _audit_hook_clear_count = 0;
+
 static int _audit_hook(const char *event, PyObject *args, void *userdata)
 {
+    assert(args && PyTuple_CheckExact(args));
     if (strcmp(event, "_testembed.raise") == 0) {
         PyErr_SetString(PyExc_RuntimeError, "Intentional error");
         return -1;
@@ -1122,6 +1125,8 @@ static int _audit_hook(const char *event, PyObject *args, void *userdata)
             return -1;
         }
         return 0;
+    } else if (strcmp(event, "cpython._PySys_ClearAuditHooks") == 0) {
+        _audit_hook_clear_count += 1;
     }
     return 0;
 }
@@ -1167,6 +1172,9 @@ static int test_audit(void)
 {
     int result = _test_audit(42);
     Py_Finalize();
+    if (_audit_hook_clear_count != 1) {
+        return 0x1000 | _audit_hook_clear_count;
+    }
     return result;
 }
 

Python/pylifecycle.c

@@ -1291,6 +1291,13 @@ finalize_interp_clear(PyThreadState *tstate)
         _PyGC_CollectNoFail();
     }
 
+    /* Clear all loghooks */
+    /* Both _PySys_Audit function and users still need PyObject, such as tuple.
+       Call _PySys_ClearAuditHooks when PyObject available. */
+    if (is_main_interp) {
+        _PySys_ClearAuditHooks(tstate);
+    }
+
     _PyGC_Fini(tstate);
 
     if (is_main_interp) {
@@ -1405,9 +1412,6 @@ Py_FinalizeEx(void)
      */
     _PyGC_CollectIfEnabled();
 
-    /* Clear all loghooks */
-    _PySys_ClearAuditHooks(tstate);
-
     /* Destroy all modules */
     _PyImport_Cleanup(tstate);