bpo-33786: Fix asynchronous generators to handle GeneratorExit in athrow() #7467
Conversation
| o->agt_state = AWAITABLE_STATE_CLOSED; | ||
| if (o->agt_args == NULL) { | ||
| /* when aclose() is called we don't want to propagate | ||
| StopAsyncIteration; just raise StopIteration, signalling | ||
| that 'aclose()' is done. */ | ||
| StopAsyncIteration or GeneratorExit; just raise |
There was a problem hiding this comment.
As far as I understand, this will make .aclose() raise StopIteration in the usual case (when generator doesn't handle the GeneratorExit). What about .athrow(), how will it behave? Shouldn't their behavior be similar to that of .close() and .throw() for sync generators, where .close() swallows the GeneratorExit and .throw() doesn't?
There was a problem hiding this comment.
As far as I understand, this will make .aclose() raise StopIteration in the usual case (when generator doesn't handle the GeneratorExit).
Keep in mind, that raising StopIteration from any of a***() methods just means that they are returning. They are all iterators.
What about .athrow(), how will it behave?
The idea here:
-
athrow()andaclose()share one implementation. -
aclose()is basically anathrow(GeneratorExit). -
When
aclose()is called and the generator propagatesGeneratorExit, it means that everything is fine and theaclose()call was successful. We raiseStopIterationto makeaclose()awaitable return. -
When
athrow()is called we don't want to trap any exceptions at all. So we raiseStopIterationonly if the asynchronous generator swallowed the thrown error and yielded (asynchronously).
This is a bit complex, I know. I explained how asynchronous generators work in detail here: https://www.python.org/dev/peps/pep-0525/#implementation-details
There was a problem hiding this comment.
Looks good.
Would you apply the following test to test_contexlib_async.py and check explicitly is the problem gone
+ @_async_test
+ async def test_async_gen(self):
+ @asynccontextmanager
+ async def ctx():
+ yield
+
+ async def gen():
+ async with ctx():
+ yield 11
+
+ ret = []
+ exc = ValueError(22)
+ with self.assertRaises(ValueError):
+ async with ctx():
+ async for val in gen():
+ ret.append(val)
+ raise exc
+
+ self.assertEqual(ret, [11])
+
Good idea. I added your regression test. |
|
Thanks @1st1 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7. |
…row() (pythonGH-7467) (cherry picked from commit 52698c7) Co-authored-by: Yury Selivanov <yury@magic.io>
|
GH-7506 is a backport of this pull request to the 3.7 branch. |
|
Thanks @1st1 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7. |
|
GH-21878 is a backport of this pull request to the 3.7 branch. |
…row() (pythonGH-7467) (cherry picked from commit 52698c7) Co-authored-by: Yury Selivanov <yury@magic.io>
Summary: This functionality was changed in python/cpython#7467. Based on Facebook D27940847
https://bugs.python.org/issue33786