gh-155619: Fix data races in the context watcher registry

Doc/c-api/contextvars.rst

@@ -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)
@@ -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
@@ -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``.

Lib/test/test_free_threading/test_context_watcher.py

@@ -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()

Misc/NEWS.d/next/Core_and_Builtins/2026-08-17-16-08-05.gh-issue-155619.Qw8Ftz.rst

@@ -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.

Modules/_testcapi/watchers.c

@@ -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))
 {
@@ -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},

Python/context.c

@@ -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);
@@ -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;
         }
     }
@@ -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;
 }