Skip to content
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

Suppression of CancelledError in CancelScope can break asynccontextmanager #463

Closed
jonathanslenders opened this issue Sep 2, 2022 · 3 comments

Comments

@jonathanslenders
Copy link
Contributor

jonathanslenders commented Sep 2, 2022

Take this piece of code:

from anyio import CancelScope
import asyncio
from contextlib import asynccontextmanager

@asynccontextmanager
async def cm():
    with CancelScope() as scope:
        scope.cancel()  # Remove this and everything works.
        await asyncio.sleep(1)
        yield scope

async def main():
    async with cm() as scope:
        pass

asyncio.run(main())

Run it causes this exception.

Traceback (most recent call last):
  File "/tmp/anyiobug.py", line 17, in <module>
    asyncio.run(main())
  File "/home/jonathan/.pyenv/versions/3.9.13/lib/python3.9/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/home/jonathan/.pyenv/versions/3.9.13/lib/python3.9/asyncio/base_events.py", line 647, in run_until_complete
    return future.result()
  File "/tmp/anyiobug.py", line 14, in main
    async with cm() as scope:
  File "/home/jonathan/.pyenv/versions/3.9.13/lib/python3.9/contextlib.py", line 183, in __aenter__
    raise RuntimeError("generator didn't yield") from None
RuntimeError: generator didn't yield

I'm wondering whether there is something fundamentally wrong with this snippet. My understanding is that a cancel scope can be cancelled at any point in time. But asynccontextmanager either expects a yield or an Exception, it doesn't get neither of them.

How would it be possible to prevent this situation? We have something similar happen in a much more complex codebase, and I'm afraid that any usage of task groups or cancel scopes within an asynccontextmanager is prone to this bug.

Here is a related issue about suppressing CancelledError: #374

@jonathanslenders
Copy link
Contributor Author

On a second thought, I think what anyio does is again correct, I'm not 100% sure what happens in our code. I hope to come with a better repro.

@smurfix
Copy link
Collaborator

smurfix commented Sep 2, 2022

The asynccontextmanager needs the function it wraps to yield exactly once (unless it raises an exception), period, end of discussion. Your code is not yielding and it doesn't raise an exception either, so …

My workaround for this problem was along these lines:

@asynccontextmanager
async def crashy():
    yielded = False
    try:
        … something
        yielded = True
        yield whatever
        … something more
    finally:
        if not yielded:
            yield None  # or raise a RuntimeError or whatever

@jonathanslenders
Copy link
Contributor Author

Thanks @smurfix, I'm closing this issue for now. This is also what I understood, the issue is not in anyio.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants