Skip to content

Conversation

@achimnol
Copy link
Contributor

@achimnol achimnol commented Jan 14, 2018

Here are my first draft PR to pass the async_generator test case provided by @njsmith.
I have reset gi_running value to 1 after gen_send_ex() and _gen_throw() calls in async generators and clear it to 0 when they close.
This PR obviously requires more test cases regarding athrow() calls, but I submit at this stage to check if my approach is correct first.

https://bugs.python.org/issue32526

@achimnol achimnol requested a review from 1st1 as a code owner January 14, 2018 17:21
@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!

@achimnol
Copy link
Contributor Author

achimnol commented Jan 14, 2018

Hm.. my changes makes too many existing tests (test_asyncgen) failing except the new test case. 😱

Async generators awaiting its inner awaitables should ALLOW send() on its returned async iterator object (as the current Python does now but gi_running becomse False) while should FORBID another asend() on itself from outside. This PR forbids both cases and thus breaks. 😞
It seems non-trivial to fix this situation... I need deeper understanding on how async-generator is implemented in CPython (coroutines and generators individually seems to be not very difficult but it looks like that async-generator mixes them up...).

I hope @1st1 and @njsmith would have some better ideas than me!
At least, my try will help me understand what's going on there even if somebody else fixes this issue.

@achimnol achimnol closed this Jan 14, 2018
@njsmith
Copy link
Contributor

njsmith commented Jan 14, 2018 via email

@1st1
Copy link
Member

1st1 commented Jan 14, 2018

I'll take a look tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants