[cobed95]

#155619

Reproduction

gh-155619 was reported from source inspection without a reproducer, so here is one first. The new test registers a watcher whose callback does nothing, then races context switches against add/clear. On unpatched main it aborts every time:

$ ./python.exe -m test test_free_threading.test_context_watcher
Assertion failed: (cb != NULL), function notify_context_watchers, file context.c
Fatal Python error: Aborted

$ PYTHON_GIL=1 ./python.exe -m test test_free_threading.test_context_watcher
== Tests result: SUCCESS ==   (2 tests, 33 ms)

Same binary, only the GIL toggled.

There are three races. PyContext_AddWatcher() claimed a slot with a plain test-then-store, so two threads could take the same slot and get the same watcher ID. It then stored the callback and set the active bit as ordinary writes, so a notifier on a weakly ordered machine could see the bit before the pointer. And notify_context_watchers() reads the bitmask and then loads the slot, which PyContext_ClearWatcher() can empty in between — that one is a plain interleaving, so no amount of memory ordering fixes it.

Fix

The fix claims slots with _Py_atomic_compare_exchange_ptr(), publishes the callback with a release store paired with an acquire load in notification, and skips a NULL callback instead of asserting on it, same as _PyDict_SendEvent(). No locks: notification runs on every context switch and does no atomic read-modify-write there. I kept the bitmask rather than scanning all eight slots because it makes the common zero-watcher case one load and a not-taken branch.

Docs

Doc/c-api/contextvars.rst had no threading guidance at all, unlike dict and type watchers, so I added some.

Testing

The new test passes 20/20 runs; test_free_threading, test_context, test_capi, test_contextlib and test_asyncio are clean on a free-threaded build (4735 tests), and the same binary with PYTHON_GIL=1 is clean too.

Benchmarks

pyperformance async_tree, async_tree_io, async_generators (asyncio runs each task step through ctx.run()), plus nbody as a control that never enters a context. Free-threaded build, baseline vs. this change:

Benchmark	Baseline	Patched
async_generators	245 ms +- 23 ms	236 ms +- 5 ms	1.04x faster, not significant
async_tree_io	374 ms +- 23 ms	363 ms +- 5 ms	1.03x faster, not significant
async_tree_none	175 ms +- 8 ms	177 ms +- 8 ms	1.01x slower, not significant
nbody (control)	77.0 ms +- 5.6 ms	76.6 ms +- 1.5 ms	1.01x faster, not significant

The default build was the same story, except async_generators came out 1.08x slower there. I think that one is noise: its spread is about 12% of the mean, t=-2.54, and the same benchmark went faster on the free-threaded build. These were plain -O3 builds with --fast on a laptop, so they answer "does this cost anything" and not much else; happy to redo them properly if it matters.

No ThreadSanitizer run from me, though test_free_threading is in TSAN_TESTS, so CI should cover the new test.

Approach discussed with @corona10 at the PyCon KR sprint.

[cobed95]

Happily waiting for reviews from @hugovk and @corona10 :)

[read-the-docs-community]

Documentation build overview

📚 cpython-previews | 🛠️ Build #34097514 | 📁 Comparing 2b13f29 against main (c3df37c)

  🔍 Preview build  

2 files changed

± c-api/contextvars.html
± whatsnew/changelog.html

[corona10]

@cobed95 Sorry for the delay, I will take a look at this week.

[florentinl]

Thank you @cobed95 for looking into the issue I reported and putting together a fix!

@corona10, would you be able to review this when you have a chance? Thanks!

[corona10]

Yeah I will do this during this sprint

[corona10]

It doesn not gurentee to set set_context_watcher_bit if CAS is not success, you may run loop here.

[corona10]

This will make issue if one thread clear watcher_id and other thread try to add watcher_id.
Seqeuntial operation should be gurenteed.

[corona10]

Please rewrite comment as concise as possible.

[corona10]

I thint that following functions should use critical section to gurentee sequetiallly consistant output.

• PyContext_ClearWatcher
• PyContext_AddWatcher

[bedevere-app]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.