gh-120321: fix thread safety of concurrently iterating over async generators - #155025
gh-120321: fix thread safety of concurrently iterating over async generators#155025kumaraditya303 wants to merge 6 commits into
Conversation
71a1b45 to
5f94aa1
Compare
| o->ags_gen->ag_running_async = 1; | ||
| do_send:; | ||
| PyObject *result = gen_send((PyObject*)o->ags_gen, arg); | ||
| result = async_gen_unwrap_value(o->ags_gen, result); |
There was a problem hiding this comment.
It looks like async_gen_unwrap_value manipulates ag_running_async non-atomically:
Lines 1922 to 1925 in fd9feab
There was a problem hiding this comment.
I don't think so, on this PR ag_running_async is not modified in async_gen_unwrap_value.
There was a problem hiding this comment.
I think we can end up in the following situation:
- T1 calls
async_gen_asend_send, successfully transitionso->ags_statetoAWAITABLE_STATE_ITER, and starts executing the generator viagen_send. - T2 calls
async_gen_asend_send, findso->ags_stateis alreadyAWAITABLE_STATE_ITER, and jumps todo_send. - T2 calls
gen_send, which returnsNULLwith an exception set because the generator is already executing. - T2 calls
async_gen_unwrap_valuewith aNULLresult, which ends up settinggen->ag_running_asyncnon-atomically.
| AWAITABLE_STATE_ITER)); | ||
|
|
||
| // INIT -> ITER transition succeeded, this is the first send. | ||
| if (!async_gen_try_claim_running(o->ags_gen)) { |
There was a problem hiding this comment.
Why is this needed? Shouldn't the logic above ensure that only one thread reaches here?
There was a problem hiding this comment.
There two possible ways to concurrently iterate an async generator, one by sharing e.g. a single asend object across threads or multiple asend objects trying to send to same generator. This is needed for the second case of multiple asend objects.
There was a problem hiding this comment.
I added a comment to clarify this.
Co-authored-by: mpage <mpage@cs.stanford.edu>
2 participants
This PR fixes thread safety of async generators similar to #142599