Repository navigation
Conversation
…ask lists When a thread exits with pending asyncio Tasks, PyThreadState_Clear() moves them onto the shared interp->asyncio_tasks_head list, but each Task still records the _Py_ThreadId() of the thread that created it. unregister_task() skips the stop-the-world pause whenever that id matches the current thread. That also happens for a later thread that reuses the exited thread's pthread_t, and for destructors that run later in PyThreadState_Clear(). The resulting unlocked llist_remove() calls on the shared list race with each other and can leave an already-removed node (next == NULL) linked into it. asyncio.all_tasks(), which asyncio.run() calls on shutdown, then crashes with SIGSEGV in _asyncio_all_tasks(). Record the unique id (PyThreadState.id) of the creating thread state instead, and only take the lock-free path while that thread state is not being cleared. Also drop the assertion in add_tasks_llist() that every linked Task is pending. A Task's state is set before unregister_task() unlinks it, and that removal can be waiting for the stop-the-world pause during which the list is walked. add_one_task() already filters out Tasks that are done.
|
All commit authors signed the Contributor License Agreement. |
|
Hi, is there anything I could do to help unblock this PR? I don't know what is the expected review time for such bug-fixes. I don't know if they are reviewed in bulk every N-weeks or if we don't hear back after a few days it's a bad sign. |
|
@kumaraditya303 I think this is related to #159073. @jblespiau you're going to need to update this to account for changes made there. |
| # them onto interp->asyncio_tasks_head. Unregistering those tasks from | ||
| # later threads (which may reuse the exited thread's OS thread ID) or | ||
| # during PyThreadState_Clear() while all_tasks() walks the task lists | ||
| # must not corrupt the shared list. |
There was a problem hiding this comment.
claude-ish comments like this and other comments below should be avoided. Maybe take a look at https://github.com/gpshead/cpython-skills/blob/main/plugins/cpython-skills/skills/style/SKILL.md#comment-quality. My mantra is something like "comments shouldn't memorialize old bugs or narrate what happens below them and should only be used to explain non-obvious nonlocal facts that are important for understanding".
When a thread exits with pending asyncio Tasks, PyThreadState_Clear() moves them onto the shared interp->asyncio_tasks_head list, but each Task still records the _Py_ThreadId() of the thread that created it. unregister_task() skips the stop-the-world pause whenever that id matches the current thread. That also happens for a later thread that reuses the exited thread's pthread_t, and for destructors that run later in PyThreadState_Clear(). The resulting unlocked llist_remove() calls on the shared list race with each other and can leave an already-removed node (next == NULL) linked into it. asyncio.all_tasks(), which asyncio.run() calls on shutdown, then crashes with SIGSEGV in _asyncio_all_tasks().
Record the unique id (PyThreadState.id) of the creating thread state instead, and only take the lock-free path while that thread state is not being cleared.
Also drop the assertion in add_tasks_llist() that every linked Task is pending. A Task's state is set before unregister_task() unlinks it, and that removal can be waiting for the stop-the-world pause during which the list is walked. add_one_task() already filters out Tasks that are done.
3.14 fix is in #159451 that we can reopen if this one is accepted.