Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions Lib/test/audit-tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,31 @@ def test_basic():
assertEqual(hook.seen[0][1], (1, 2, 3))


def test_concurrent_add_hook():
from test.support import threading_helper

# gh-154431: Race the first hook registrations against audit events.
seen = []

def add_hook(index):
def hook(event, args):
if event == "test.concurrent_add_hook":
seen.append(index)

sys.addaudithook(hook)

def audit():
for _ in range(2000):
sys.audit("test.concurrent_audit")

workers = [lambda i=i: add_hook(i) for i in range(24)]
workers.extend([audit] * 8)
threading_helper.run_concurrently(workers)

sys.audit("test.concurrent_add_hook")
assertEqual(sorted(seen), list(range(24)))


def test_block_add_hook():
# Raising an exception should prevent a new hook from being added,
# but will not propagate out.
Expand Down
5 changes: 5 additions & 0 deletions Lib/test/test_audit.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from test import support
from test.support import import_helper
from test.support import os_helper
from test.support import threading_helper


if not hasattr(sys, "addaudithook") or not hasattr(sys, "audit"):
Expand Down Expand Up @@ -52,6 +53,10 @@ def run_python(self, *args, expect_stderr=False):
def test_basic(self):
self.do_test("test_basic")

@threading_helper.requires_working_threading()
def test_concurrent_add_hook(self):
self.do_test("test_concurrent_add_hook")

def test_block_add_hook(self):
self.do_test("test_block_add_hook")

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix a data race when :func:`sys.addaudithook` is called concurrently in the
free-threaded build.
26 changes: 15 additions & 11 deletions Python/sysmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -239,8 +239,12 @@ should_audit(PyInterpreterState *interp)
if (!interp) {
return 0;
}
// interp->audit_hooks can only ever be NULL very early during initialization
// or very late during finalization.
int interp_has_audit_hooks = (interp->audit_hooks != NULL
&& PyList_GET_SIZE(interp->audit_hooks) > 0);
return (interp->runtime->audit_hooks.head
|| interp->audit_hooks
|| interp_has_audit_hooks
|| PyDTrace_AUDIT_ENABLED());
}

Expand Down Expand Up @@ -310,7 +314,8 @@ sys_audit_tstate(PyThreadState *ts, const char *event,
}

/* Call interpreter hooks */
if (is->audit_hooks) {
PyObject *audit_hooks = is->audit_hooks;
if (audit_hooks != NULL && PyList_GET_SIZE(audit_hooks) > 0) {
eventName = PyUnicode_FromString(event);
if (!eventName) {
goto exit;
Expand Down Expand Up @@ -540,15 +545,7 @@ sys_addaudithook_impl(PyObject *module, PyObject *hook)
}

PyInterpreterState *interp = tstate->interp;
if (interp->audit_hooks == NULL) {
interp->audit_hooks = PyList_New(0);
if (interp->audit_hooks == NULL) {
return NULL;
}
/* Avoid having our list of hooks show up in the GC module */
PyObject_GC_UnTrack(interp->audit_hooks);
}

assert(interp->audit_hooks != NULL);
if (PyList_Append(interp->audit_hooks, hook) < 0) {
return NULL;
}
Expand Down Expand Up @@ -4367,6 +4364,13 @@ _PySys_Create(PyThreadState *tstate, PyObject **sysmod_p)

PyInterpreterState *interp = tstate->interp;

PyObject *audit_hooks = PyList_New(0);
if (audit_hooks == NULL) {
goto error;
}
_PyObject_GC_UNTRACK(audit_hooks);
interp->audit_hooks = audit_hooks;

PyObject *modules = _PyImport_InitModules(interp);
if (modules == NULL) {
goto error;
Expand Down
Loading