From: Steve Dower <steve.dower@python.org>
Date: Fri, 3 Jul 2020 23:04:22 +0000 (+0100)
Subject: bpo-41162: Clear audit hooks later during finalization (GH-21222)
X-Git-Tag: v3.8.4~20
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b9e288cc1bfd583e887f784e38d9c511b43c0c3a;p=thirdparty%2FPython%2Fcpython.git

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

Co-authored-by: Konge <zkonge@outlook.com>
---

diff --git a/Lib/test/audit-tests.py b/Lib/test/audit-tests.py
index a58395b068b3..ee6fc93351b7 100644
--- a/Lib/test/audit-tests.py
+++ b/Lib/test/audit-tests.py
@@ -44,28 +44,6 @@ class TestHook:
             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
 
diff --git a/Lib/test/test_audit.py b/Lib/test/test_audit.py
index f405c6923979..f79edbc4bd0d 100644
--- a/Lib/test/test_audit.py
+++ b/Lib/test/test_audit.py
@@ -51,22 +51,6 @@ class AuditTest(unittest.TestCase):
     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")
 
diff --git a/Misc/NEWS.d/next/Security/2020-07-03-20-41-29.bpo-41162.tb8pVj.rst b/Misc/NEWS.d/next/Security/2020-07-03-20-41-29.bpo-41162.tb8pVj.rst
new file mode 100644
index 000000000000..f0333ac4a7bb
--- /dev/null
+++ b/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.
\ No newline at end of file
diff --git a/Programs/_testembed.c b/Programs/_testembed.c
index b98a38a1ba67..460d70ccadef 100644
--- a/Programs/_testembed.c
+++ b/Programs/_testembed.c
@@ -1106,8 +1106,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;
@@ -1116,6 +1119,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;
 }
@@ -1161,6 +1166,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;
 }
 
diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
index 27cebf33da54..dc2d13db419e 100644
--- a/Python/pylifecycle.c
+++ b/Python/pylifecycle.c
@@ -1229,13 +1229,6 @@ Py_FinalizeEx(void)
         /* nothing */;
 #endif
 
-    /* Clear all loghooks */
-    /* We want minimal exposure of this function, so define the extern
-     * here. The linker should discover the correct function without
-     * exporting a symbol. */
-    extern void _PySys_ClearAuditHooks(void);
-    _PySys_ClearAuditHooks();
-
     /* Destroy all modules */
     PyImport_Cleanup();
 
@@ -1306,6 +1299,13 @@ Py_FinalizeEx(void)
     /* Clear interpreter state and all thread states. */
     PyInterpreterState_Clear(interp);
 
+    /* Clear all loghooks */
+    /* We want minimal exposure of this function, so define the extern
+     * here. The linker should discover the correct function without
+     * exporting a symbol. */
+    extern void _PySys_ClearAuditHooks(void);
+    _PySys_ClearAuditHooks();
+
     /* Now we decref the exception classes.  After this point nothing
        can raise an exception.  That's okay, because each Fini() method
        below has been checked to make sure no exceptions are ever