Skip to content

asyncio.Runner allows async generators to skip gracefull finalization #95584

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
graingert opened this issue Aug 3, 2022 · 10 comments
Closed
Labels
3.11 only security fixes 3.12 only security fixes pending The issue will be closed if no feedback is provided topic-asyncio type-bug An unexpected behavior, bug, or error

Comments

@graingert
Copy link
Contributor

reproducer:

import gc
import asyncio
import weakref

async def agenfn():
    for i in range(100):
        try:
            yield i
        finally:
            await asyncio.sleep(0)


with asyncio.Runner() as runner:
    agen = agenfn()
    print(runner.run(anext(agen)))
    print(runner.run(anext(agen)))
    print(runner.run(anext(agen)))
    agen_wr = weakref.ref(agen)
    del agen
    gc.collect()
    assert agen_wr() is None


async def run_through():
    agen = agenfn()
    print(await anext(agen))
    print(await anext(agen))
    print(await anext(agen))
    agen_wr = weakref.ref(agen)
    del agen
    gc.collect()
    assert agen_wr() is None

asyncio.run(run_through())
gc.collect()

output:

0
1
2
Exception ignored in: <async_generator object agenfn at 0x7f29a019d000>
Traceback (most recent call last):
  File "/home/graingert/projects/gc_agen_runner.py", line 19, in <module>
    del agen
        ^^^^
RuntimeError: async generator ignored GeneratorExit
0
1
2
@graingert graingert added type-bug An unexpected behavior, bug, or error topic-asyncio 3.11 only security fixes 3.12 only security fixes labels Aug 3, 2022
@graingert
Copy link
Contributor Author

graingert commented Aug 4, 2022

I think this is a fundamental problem with allowing an event loop to be paused and resumed.

currently pausing and resuming a loop can impact other threads relying on the main thread loop to handle processes
and as above async generator cleanup relies on an eventloop that remains in full control of the thread until it closes and shuts down, eg:

try:
    loop.run_forever()
finally:
    loop.run_until_complete(loop.shutdown_asyncgens())
    loop.close()

it makes sense for low-level hooks like loop.run_until_complete and loop.close to permit this, but high-level APIs shouldn't expose users to these sorts of edge cases

cc @njsmith

@kumaraditya303
Copy link
Contributor

Is this an issue in practice though? I don't see any point in using a async generator across multiple runs. It should be iterated over once in the cycle.

@kumaraditya303 kumaraditya303 added the pending The issue will be closed if no feedback is provided label Nov 26, 2022
@ezio-melotti ezio-melotti moved this to Todo in asyncio Nov 26, 2022
@gvanrossum
Copy link
Member

@graingert Do you have a fix in mind?

@kumaraditya303
Copy link
Contributor

FWIW I consider this to be a misuse of the API here not as a bug, you are not supposed to create async generator without a running event loop otherwise you end up not triggering async hooks. @graingert Do you have any use case for this? otherwise I'll close this a won't fix. See my comment #95584 (comment)

@gvanrossum
Copy link
Member

you are not supposed to create async generator without a running event loop otherwise you end up not triggering async hooks.

But that feels like an arbitrary restriction. I could imagine doing exactly that to control some test case, for example. (The advantage of doing it this way rather than the other way would be that the test itself can remain synchronous.)

@kumaraditya303
Copy link
Contributor

But that feels like an arbitrary restriction. I could imagine doing exactly that to control some test case, for example. (The advantage of doing it this way rather than the other way would be that the test itself can remain synchronous.)

I guess you are not aware of how async gen initialization and finalization works. See https://docs.python.org/3/library/sys.html#sys.set_asyncgen_hooks, using it without a running loop is inviting issues.

@gvanrossum
Copy link
Member

guess you are not aware of how async gen initialization and finalization works.

That sounds a little condescending. :-)

I sometimes try to see things from the POV of an unsophisticated user (which honestly I still am, most of the time :-).

The docs there don't actually explain a thing, the semantics are in PEP 525. I could look that up, but basically my point is that nobody who isn't an asyncio implementation expert understands how async generators really work, so the restrictions feel arbitrary (even though I remember that when I approved the PEP I understood why they existed and couldn't think of a better way either).

At the very least the docs should explain the restrictions of async generators. Not sure where that should go though -- the sys function seems pretty obscure, but the language reference doesn't usually talk about specific async frameworks like asyncio, and the asyncio docs don't have a place to discuss this since when you use it as intended they "just work".

@kumaraditya303
Copy link
Contributor

I sometimes try to see things from the POV of an unsophisticated user (which honestly I still am, most of the time :-).

I imagine that this isn't something an unsophisticated user could ever hit. You can only ever hit this if you are trying hard to do it. The general case is that an agen is created inside a coroutine, it is exhausted and then it is closed. If you are creating it outside a running event loop, and using it across multiple event loops or asyncio.run cycles then you cannot expect it to work at all and in fact if it will work, it will promote wrong idiom of using agen.

At the very least the docs should explain the restrictions of async generators. Not sure where that should go though -- the sys function seems pretty obscure, but the language reference doesn't usually talk about specific async frameworks like asyncio, and the asyncio docs don't have a place to discuss this since when you use it as intended they "just work".

+1 for better documentation.

@kumaraditya303
Copy link
Contributor

I created #100108 to add better documentation for async generators.

@kumaraditya303
Copy link
Contributor

Closing as #100108 covers this.

@kumaraditya303 kumaraditya303 closed this as not planned Won't fix, can't repro, duplicate, stale Apr 19, 2023
@github-project-automation github-project-automation bot moved this from Todo to Done in asyncio Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes 3.12 only security fixes pending The issue will be closed if no feedback is provided topic-asyncio type-bug An unexpected behavior, bug, or error
Projects
Status: Done
Development

No branches or pull requests

3 participants