diff --git a/Lib/test/audit-tests.py b/Lib/test/audit-tests.py index a58395b068b..ee6fc93351b 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 f405c692397..f79edbc4bd0 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 00000000000..f0333ac4a7b --- /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 b60d70be5f7..5aad16a6f7c 100644 --- a/Programs/_testembed.c +++ b/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; } diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 3ce2c41ef1f..2d219a4a3a8 100644 --- a/Python/pylifecycle.c +++ b/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);