| msg325699 - (view) |
Author: Devin Fee (dfee) |
Date: 2018-09-19 00:27 |
| I expected an async-generator's aclose() method to cancel it's __anext__(). Instead, the task isn't cancelled, and (it seems) manually cleanup is necessary. @pytest.mark.asyncio async def test_aclose_cancels(): done = False event = asyncio.Event() agen = None async def make_agen(): try: await event.wait() raise NotImplementedError() yield # pylint: disable=W0101, unreachable finally: nonlocal done done = True async def run(): nonlocal agen agen = make_agen() await agen.__anext__() task = asyncio.ensure_future(run()) await asyncio.sleep(.01) await agen.aclose() await asyncio.sleep(.01) assert done assert task.done() and task.exception() Looking at the tests for CPython, the implementation seems to expect it, but I'm unclear if PEP-525 actually intends for this to be the case: https://github.com/python/cpython/blob/e42b705188271da108de42b55d9344642170aa2b/Lib/test/test_asyncgen.py#L657 Alternatively, maybe I shouldn't be calling aclose, and instead I should be cancelling the task: @pytest.mark.asyncio async def test_cancel_finalizes(): done = False event = asyncio.Event() agen = None async def make_agen(): try: await event.wait() raise NotImplementedError() yield # pylint: disable=W0101, unreachable finally: nonlocal done done = True async def run(): nonlocal agen agen = make_agen() await agen.__anext__() task = asyncio.ensure_future(run()) await asyncio.sleep(.01) task.cancel() await asyncio.sleep(.01) assert done assert task.done() So the "bug" here is one of PEP-525 clarification. |
|
|
| msg325704 - (view) |
Author: Devin Fee (dfee) |
Date: 2018-09-19 06:12 |
| I've worked around this problem by doing something like this: async def close_cancelling(agen): while True: try: task = asyncio.ensure_future(agen.__anext__()) await task yield task.result() except (GeneratorExit, StopAsyncIteration): await agen.aclose() task.cancel() break async def run(): try: async for v in close_cancelling(agen): received.append(v) except asyncio.CancelledError: # handle finalization pass Though, I'm still convinced that `aclose()` should call raise `StopAsyncIteration` on `agen.ag_await` |
|
|
| msg325920 - (view) |
Author: Yury Selivanov (yselivanov) *  |
Date: 2018-09-20 18:45 |
| Interesting. Nathaniel, do you have this problem in Trio (aclose() not cancelling an anext()) or cancel scopes solve this problem somehow? |
|
|
| msg325928 - (view) |
Author: Nathaniel Smith (njs) * |
Date: 2018-09-20 20:34 |
| Part of the issue here is the one discussed in bpo-30773 / bpo-32526: async generators allow themselves to be re-entered while another asend/athrow/aclose call is in progress, and it causes weird and confusing results. So at a minimum, trying to call aclose while another task is calling asend should make aclose raise an error saying that the generator is already busy. Of course the OP wants to go further and have aclose actually trigger a cancellation of any outstanding asend/athrow. I see the intuition here, but unfortunately I don't think this can work. Doing a cancellation requires some intimate knowledge of the coroutine runner. You can't just throw in an exception; you also have to unwind the task state. (Eg in the example with 'event.wait', you need to somehow tell the event object that this task is no longer waiting for it, so it shouldn't be notified when the event occurrs.) So I think we just need to fix ag_running and then recommend people find other ways to interrupt running async generators. |
|
|
| msg325929 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2018-09-20 20:36 |
| > So I think we just need to fix ag_running and then recommend people find other ways to interrupt running async generators. Agree. Speaking of fixing ag_running, could you please review my PR: https://github.com/python/cpython/pull/7468 I can then commit it and maybe get this fixed in 3.7.1. |
|
|
| msg375739 - (view) |
Author: Dima Tisnek (Dima.Tisnek) * |
Date: 2020-08-21 00:51 |
| https://github.com/python/cpython/pull/7468 (prohibit asend/etc. reentrance) was merged ~a year ago. Perhaps it's time to restart the original discussion, whether `aclose()` should cancel pending `anext`. |
|
|
| msg375742 - (view) |
Author: Nathaniel Smith (njs) * |
Date: 2020-08-21 02:58 |
| > Perhaps it's time to restart the original discussion, whether `aclose()` should cancel pending `anext`. I'm still not aware of any way to make this work, technically. So it's a moot point unless someone has a proposal. |
|
|
| msg375743 - (view) |
Author: Dima Tisnek (Dima.Tisnek) * |
Date: 2020-08-21 03:12 |
| Then perhaps the issue should be closed 🤔 |
|
|