classification
Title: aclose() doesn't stop raise StopAsyncIteration / GeneratorExit to __anext__()
Type: Stage:
Components: asyncio Versions: Python 3.7, Python 3.6
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: asvetlov, cito, dfee, njs, yselivanov
Priority: normal Keywords:

Created on 2018-09-19 00:27 by dfee, last changed 2018-09-20 21:58 by cito.

Messages (5)
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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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.
History
Date User Action Args
2018-09-20 21:58:50citosetnosy: + cito
2018-09-20 20:36:06yselivanovsetmessages: + msg325929
2018-09-20 20:34:17njssetmessages: + msg325928
2018-09-20 18:45:35yselivanovsetnosy: + njs
messages: + msg325920
2018-09-19 16:23:18dfeesetversions: + Python 3.6
2018-09-19 06:12:34dfeesetmessages: + msg325704
2018-09-19 00:27:26dfeecreate