Skip to content

Commit

Permalink
Fix thread safety of suspended coroutines
Browse files Browse the repository at this point in the history
Summary:
Currently we handle `suspended_coroutines` in a non-thread safe way.  When we're starting a co-routine we'll use this linked list to see if a task is already suspended.  But we then go and run arbitrary Python code after putting an item on this list, and that can swap out the GIL.  Then another task can go through this same code path, stomping on the linked list.  Because this is a effectively a thread-local operation I just mark variable as being thread-local to fix it.  We can't really upstream it in this state (can't rely on optional C11 features), but we can't really upstream it in the previous state either.  So this fix seems reasonable for now.

I also cleaned up `_is_coro_suspended` so that we treat `Ci_PyGen_IsSuspended` as being definitive and don't consult the list.

Reviewed By: AlbertDachiChen

Differential Revision: D44713397

fbshipit-source-id: f4c6eb2b055a482354ceb2b489fc57918965cc69
  • Loading branch information
DinoV authored and facebook-github-bot committed Apr 11, 2023
1 parent 3d4189e commit 219694c
Showing 1 changed file with 3 additions and 4 deletions.
7 changes: 3 additions & 4 deletions Modules/_asynciomodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ typedef struct _SuspendedCoroNode {
// it is not that trivial. In order to solve this we
// store coroutine in the linked list before calling
// 'create_task' and remove it afterwards.
static _SuspendedCoroNode *suspended_coroutines = NULL;
_Thread_local _SuspendedCoroNode *suspended_coroutines = NULL;

typedef struct {
// used to avoid allocations for cases <=64 bit
Expand Down Expand Up @@ -3528,10 +3528,9 @@ leave_task(PyObject *loop, PyObject *task)
static inline int
_is_coro_suspended(PyObject *coro)
{
if ((PyCoro_CheckExact(coro) || PyGen_CheckExact(coro)) &&
Ci_PyGen_IsSuspended((PyGenObject *)coro)) {
if (PyCoro_CheckExact(coro) || PyGen_CheckExact(coro)) {
// do not automatically schedule suspended coroutines
return 1;
return Ci_PyGen_IsSuspended((PyGenObject *)coro);
}
_SuspendedCoroNode *n = suspended_coroutines;
while (n != NULL) {
Expand Down

0 comments on commit 219694c

Please sign in to comment.