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
18 changes: 18 additions & 0 deletions Doc/c-api/contextvars.rst
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ Context object management functions:
In case of error (e.g. no more watcher IDs available),
return ``-1`` and set an exception.

This function may be called concurrently with other calls to
:c:func:`PyContext_AddWatcher` and :c:func:`PyContext_ClearWatcher`, and
while other threads are switching contexts. Each successful call returns a
distinct watcher ID.

.. versionadded:: 3.14

.. c:function:: int PyContext_ClearWatcher(int watcher_id)
Expand All @@ -117,6 +122,14 @@ Context object management functions:
Return ``0`` on success, or ``-1`` and set an exception on error
(e.g. if the given *watcher_id* was never registered.)

After this returns, the callback is not selected for context switches that
begin later. It does not wait for callbacks already in progress: in the
:term:`free-threaded <free threading>` build a callback selected on another
thread just before the watcher was cleared may still be running, or may
still be about to run. State used by the callback must therefore remain
valid until the caller has established, by its own means, that no
invocation can still be in flight.

.. versionadded:: 3.14

.. c:type:: PyContextEvent
Expand All @@ -135,6 +148,11 @@ Context object management functions:
Context object watcher callback function. The object passed to the callback
is event-specific; see :c:type:`PyContextEvent` for details.

The callback is invoked by the thread whose current context changed. In the
:term:`free-threaded <free threading>` build several threads may therefore
run the same callback at the same time, and the callback is responsible for
synchronizing any state it shares between those invocations.

If the callback returns with an exception set, it must return ``-1``; this
exception will be printed as an unraisable exception using
:c:func:`PyErr_FormatUnraisable`. Otherwise it should return ``0``.
Expand Down
69 changes: 69 additions & 0 deletions Lib/test/test_free_threading/test_context_watcher.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import contextvars
import unittest

from test.support import import_helper, threading_helper

_testcapi = import_helper.import_module("_testcapi")

ITERS = 1000
NTHREADS = 8


@threading_helper.requires_working_threading()
class TestContextWatcherThreadSafety(unittest.TestCase):
# gh-155619: the per-interpreter context watcher registry
# (PyInterpreterState.context_watchers plus the active_context_watchers
# bitmask) is read and written without synchronization. Every callback
# used here is a no-op, so any failure is CPython's registry rather than
# the test's own bookkeeping.

def test_concurrent_add_clear_watchers(self):
"""Race AddWatcher against ClearWatcher.

Both scan/modify the callback array and do a read-modify-write on
the bitmask, so two callers can claim the same slot or lose a
bitmask update.
"""
results = []

def worker():
for _ in range(ITERS):
try:
wid = _testcapi.add_noop_context_watcher()
except RuntimeError:
continue # all CONTEXT_MAX_WATCHERS slots taken
self.assertGreaterEqual(wid, 0)
results.append(wid)
_testcapi.clear_context_watcher(wid)

threading_helper.run_concurrently(worker, NTHREADS)
self.assertGreater(len(results), 0)

def test_clear_watcher_races_notification(self):
"""Race ClearWatcher against notification.

notify_context_watchers() snapshots the bitmask, then loads
context_watchers[i], which ClearWatcher may have set to NULL in
between. A debug build reaches assert(cb != NULL); a release build
may call a NULL or stale function pointer.
"""
def switcher():
# Context().run() enters and exits a context, and each switch
# dispatches to every active watcher on this thread.
for _ in range(ITERS):
contextvars.Context().run(lambda: None)

def churner():
for _ in range(ITERS):
try:
wid = _testcapi.add_noop_context_watcher()
except RuntimeError:
continue
_testcapi.clear_context_watcher(wid)

workers = [switcher, churner] * (NTHREADS // 2)
threading_helper.run_concurrently(workers)


if __name__ == "__main__":
unittest.main()
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Fix data races in the per-interpreter context watcher registry on
:term:`free-threaded <free threading>` builds. :c:func:`PyContext_AddWatcher`
now claims a slot with an atomic compare-and-swap, so concurrent callers can
no longer select the same slot or receive the same watcher ID, and
notification loads each callback with an acquire load and skips one that
:c:func:`PyContext_ClearWatcher` has retired concurrently. Previously a
context switch racing with :c:func:`PyContext_ClearWatcher` could reach an
assertion in a debug build, or call a ``NULL`` function pointer in a release
build.
14 changes: 14 additions & 0 deletions Modules/_testcapi/watchers.c
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,19 @@ clear_context_watcher(PyObject *self, PyObject *watcher_id)
Py_RETURN_NONE;
}

/* Register a watcher whose callback touches no shared state, so that a
concurrency test exercises only CPython's watcher registry and cannot
race on the test harness's own bookkeeping. */
static PyObject *
add_noop_context_watcher(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args))
{
int watcher_id = PyContext_AddWatcher(noop_context_event_handler);
if (watcher_id < 0) {
return NULL;
}
return PyLong_FromLong(watcher_id);
}

static PyObject *
clear_context_stack(PyObject *Py_UNUSED(self), PyObject *Py_UNUSED(args))
{
Expand Down Expand Up @@ -864,6 +877,7 @@ static PyMethodDef test_methods[] = {

// Code object watchers.
{"add_context_watcher", add_context_watcher, METH_O, NULL},
{"add_noop_context_watcher", add_noop_context_watcher, METH_NOARGS, NULL},
{"clear_context_watcher", clear_context_watcher, METH_O, NULL},
{"clear_context_stack", clear_context_stack, METH_NOARGS, NULL},
{"get_context_switches", get_context_switches, METH_O, NULL},
Expand Down
58 changes: 48 additions & 10 deletions Python/context.c
Original file line number Diff line number Diff line change
Expand Up @@ -163,14 +163,19 @@ notify_context_watchers(PyThreadState *ts, PyContextEvent event, PyObject *ctx)
assert(Py_REFCNT(ctx) > 0);
PyInterpreterState *interp = ts->interp;
assert(interp->_initialized);
uint8_t bits = interp->active_context_watchers;
uint8_t bits = FT_ATOMIC_LOAD_UINT8_RELAXED(interp->active_context_watchers);
int i = 0;
while (bits) {
assert(i < CONTEXT_MAX_WATCHERS);
if (bits & 1) {
PyContext_WatchCallback cb = interp->context_watchers[i];
assert(cb != NULL);
if (cb(event, ctx) < 0) {
// Pairs with the release store in PyContext_AddWatcher(), so that
// observing the active bit implies observing the callback pointer
// (and whatever the registering thread published before it).
PyContext_WatchCallback cb =
FT_ATOMIC_LOAD_PTR_ACQUIRE(interp->context_watchers[i]);
// PyContext_ClearWatcher() may have cleared this slot after the
// bitmask was read, so the callback can legitimately be NULL.
if (cb != NULL && cb(event, ctx) < 0) {
PyErr_FormatUnraisable(
"Exception ignored in %s watcher callback for %R",
context_event_name(event), ctx);
Expand All @@ -182,16 +187,45 @@ notify_context_watchers(PyThreadState *ts, PyContextEvent event, PyObject *ctx)
}


static inline void
set_context_watcher_bit(PyInterpreterState *interp, int watcher_id)
{
uint8_t bit = (uint8_t)(1 << watcher_id);
#ifdef Py_GIL_DISABLED
(void)_Py_atomic_or_uint8(&interp->active_context_watchers, bit);
#else
interp->active_context_watchers |= bit;
#endif
}


static inline void
clear_context_watcher_bit(PyInterpreterState *interp, int watcher_id)
{
uint8_t bit = (uint8_t)(1 << watcher_id);
#ifdef Py_GIL_DISABLED
(void)_Py_atomic_and_uint8(&interp->active_context_watchers, (uint8_t)~bit);
#else
interp->active_context_watchers &= (uint8_t)~bit;
#endif
}


int
PyContext_AddWatcher(PyContext_WatchCallback callback)
{
PyInterpreterState *interp = _PyInterpreterState_GET();
assert(interp->_initialized);

for (int i = 0; i < CONTEXT_MAX_WATCHERS; i++) {
if (!interp->context_watchers[i]) {
interp->context_watchers[i] = callback;
interp->active_context_watchers |= (1 << i);
// Claim the slot with a single atomic step: a plain test-then-store
// lets two threads select the same slot and return the same ID.
// Losing the race means the slot is taken, so just try the next one.
PyContext_WatchCallback expected = NULL;
if (_Py_atomic_compare_exchange_ptr(&interp->context_watchers[i],
&expected, callback)) {
// Publish the callback before advertising the slot as active.
set_context_watcher_bit(interp, i);
return i;
}
}
Expand All @@ -210,12 +244,16 @@ PyContext_ClearWatcher(int watcher_id)
PyErr_Format(PyExc_ValueError, "Invalid context watcher ID %d", watcher_id);
return -1;
}
if (!interp->context_watchers[watcher_id]) {
if (FT_ATOMIC_LOAD_PTR_RELAXED(interp->context_watchers[watcher_id]) == NULL) {
PyErr_Format(PyExc_ValueError, "No context watcher set for ID %d", watcher_id);
return -1;
}
interp->context_watchers[watcher_id] = NULL;
interp->active_context_watchers &= ~(1 << watcher_id);
// Stop notification selecting this slot before retiring the callback, so
// the window in which a notifier sees the bit but loads NULL is as short
// as possible. That window is benign: notify_context_watchers() skips a
// NULL callback.
clear_context_watcher_bit(interp, watcher_id);
FT_ATOMIC_STORE_PTR_RELEASE(interp->context_watchers[watcher_id], NULL);
return 0;
}

Expand Down
Loading