aclose() doesn't stop raise StopAsyncIteration / GeneratorExit to __anext__()

[dfee]

BPO	34730
Nosy	@Cito, @njsmith, @asvetlov, @dimaqq, @1st1, @dfee

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2018-09-19.00:27:25.958>
labels = ['3.10', 'expert-asyncio']
title = "aclose() doesn't stop raise StopAsyncIteration / GeneratorExit to __anext__()"
updated_at = <Date 2020-08-21.03:12:19.223>
user = 'https://github.com/dfee'

bugs.python.org fields:

activity = <Date 2020-08-21.03:12:19.223>
actor = 'Dima.Tisnek'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['asyncio']
creation = <Date 2018-09-19.00:27:25.958>
creator = 'dfee'
dependencies = []
files = []
hgrepos = []
issue_num = 34730
keywords = []
message_count = 8.0
messages = ['325699', '325704', '325920', '325928', '325929', '375739', '375742', '375743']
nosy_count = 6.0
nosy_names = ['cito', 'njs', 'asvetlov', 'Dima.Tisnek', 'yselivanov', 'dfee']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue34730'
versions = ['Python 3.10']

[dfee]

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:

cpython/Lib/test/test_asyncgen.py

Line 657 in e42b705

# Silence ResourceWarnings

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.

[dfee]

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

[1st1]

Interesting.

Nathaniel, do you have this problem in Trio (aclose() not cancelling an anext()) or cancel scopes solve this problem somehow?

[njsmith]

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.

[1st1]

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: #7468 I can then commit it and maybe get this fixed in 3.7.1.

[dimaqq]

#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.

[njsmith]

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.

[dimaqq]

Then perhaps the issue should be closed 🤔

[kumaraditya303]

IMO we should not change anything here but recommend the users to not close async generators concurrently with docs. Doing any change here is going to be both complex probably backwards incompatible as users might be relying on the current behavior.

[gvanrossum]

Sounds good. I agree that the behavior by now is ingrained and will be hard to change without breaking working user code, and not really worth it. So let's document it.

[kumaraditya303]

Documentation will be taken care of in #100108