Repository navigation
Audit asyncio thread safety #128002
Description
The following functions needs to be made thread safe and atomic for free-threading for both pure python and C implementation:
-
asyncio._enter_task -
asyncio._leave_task -
asyncio._register_task -
asyncio._unregister_task -
asyncio._swap_current_task -
asyncio.current_task -
asyncio.all_tasks
Note that some of these were made thread safe in C impl #120974
The following classes needs to be thread safe for free-threading in C implementation:
-
asyncio.Task -
asyncio.Future
Both of these classes are documented to be not thread safe but currently calling methods on these classes from multiple threads can crash the interpreter. The pure python implementation cannot crash the interpreter when called from multiple threads so changes are only needed for C implementation. Before making these thread safe in C I would gather some numbers for how much a difference the C implementation makes in free-threading and if it isn't much we can just disable the C extension for free-threading.
cc @colesbury
Linked PRs
- gh-128002: fix many thread safety issues in asyncio #128147
- gh-128002: use internal llist implementation for asyncio tasks #128256
- gh-128002: add more thread safety tests for asyncio #128480
- gh-128002: fix
asyncio.all_tasksagainst concurrent deallocations of tasks #128541 - gh-128002: use per threads tasks linked list in asyncio #128869
- gh-128002: use
_PyObject_SetMaybeWeakrefwhen creating tasks in asyncio #128885 - gh-128002: fix
test_all_tasks_different_threadin asyncio #129267 - gh-128002: simplify all_tasks to use
PyList_Extendinstead of manual iteration #129942 - gh-128002: add fast path for native tasks in
asyncio.all_tasks#129943 - gh-128002: optimistically remove tasks from linked list when finished #129995
- gh-128002: use efficient linked list impl for eager tasks in asyncio #130518
- gh-128002: add test_asyncio.test_free_threading to tsan tests #131106
- gh-128002: fix tsan race in closing socket in test_ssl #131397
- gh-128002: fix tsan race in closing socket in test_sslproto #131399
- gh-128002: run all asyncio tests with tsan CI #131797
- [3.14] gh-128002: add what's new docs for
asyncio(GH-134324) #134362 - gh-128002: add what's new docs for asyncio #134324
Activity
colesbury commentedon Dec 16, 2024
You may be already planning on this, but I think it'd be helpful to have a summary of the current thread safety issues in these classes and functions.
kumaraditya303 commentedon Dec 18, 2024
I have gathered data of performance of pure python implementation and C implementation.
Details
Without free-threading
With free-threading
From this data, it looks like there's on average slowdown of 1.50x when disabling the C implementation as such it is too large of performance loss to consider dropping the C implementation even in case of free-threading.
I was hoping that the difference would be less so we could disable the C implementation for free-threading and reduce the changes but it isn't worth the perf loss.
Some thread safety Issues in C implementation
Here's an outline of some of the thread safety issues, I'll be adding more to this list.
Critical sections needs to be added methods of
asyncio.Taskandasyncio.Futureotherwise they can crash the interpreter in C implementation. Even though tasks are documented to be not thread safe but still it shouldn't crash the interpreter. Example: tasks could be concurrently cancelled while it's stepping through the coroutine in another thread. The cancellation counter isn't atomic so it could lead to possible loss of cancellations requests.cpython/Modules/_asynciomodule.c
Lines 2363 to 2373 in bad3cde
The
eager_tasksset could be concurrently mutated while it's being iterated over in another thread inasyncio.all_tasks.cpython/Modules/_asynciomodule.c
Lines 3606 to 3608 in bad3cde
asyncio.Handleneeds to be thread safe because it can be called from any thread withloop.call_soon_threadsafe.The internal asyncio linked list uses borrowed references to task as such can be concurrently deallocated while list is being read in another thread as deallocation doesn't hold the state lock.
Performance bottlenecks in free-threading
Consider an async server which uses multiple threads to run one independent loop per thread.
current_tasks,eager_tasks,non_asyncio_tasksetc which would perform poorly with free-threading. Moving these to per thread would help in performance scaling.itamaro commentedon Dec 18, 2024
I vaguely remember past discussions about moving the global state to live on the loop. I don't remember what was the conclusion (or if there was a conclusion), but if we do that, that would resolve most thread-safety issues, wouldn't it?
kumaraditya303 commentedon Dec 18, 2024
That has backwards compatibility issues with custom event loops, currently asyncio handles it for all loops. Moving things to event loop may improve performance for free-threading but will causes performance loss in normal builds and in cases where just a single loop runs because of extra attribute lookups and indirection and we cannot use non-python data structures like the new double linked-lists implementation.
gh-128002: use internal llist implementation for asyncio tasks (#128256)
gh-128002: fix many thread safety issues in asyncio (#128147)
pythongh-128002: fix many thread safety issues in asyncio (python#128147
pythongh-128002: use internal llist implementation for asyncio tasks (p…
pythongh-128002: fix many thread safety issues in asyncio (python#128147
gh-128002: fix `asyncio.all_tasks` against concurrent deallocations o…
gh-128002: add more thread safety tests for asyncio (#128480)
18 remaining items
gh-128002: fix tsan race in closing socket in `test_ssl` (#131397)
gh-128002: fix tsan race in closing socket in `test_sslproto` (#131399)
pythongh-128002: fix tsan race in closing socket in `test_ssl` (pytho…
pythongh-128002: fix tsan race in closing socket in `test_sslproto` (p…
gh-128002: run all asyncio tests with tsan CI (#131797)
gh-128002: add what's new docs for `asyncio` (#134324)
pythongh-128002: add what's new docs for `asyncio` (pythonGH-134324)
[3.14] gh-128002: add what's new docs for `asyncio` (GH-134324) (#134362
pythongh-128002: add what's new docs for `asyncio` (python#134324)
pythongh-128002: add what's new docs for `asyncio` (python#134324)
pythongh-128002: add what's new docs for `asyncio` (python#134324)
[Python] Remove use of asyncio.all_tasks on channel close to support …
[Python] Remove use of asyncio.all_tasks on channel close to support …