Skip to content

async generator missing unawaited coroutine warning #89091

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 mannequin opened this issue Aug 16, 2021 · 18 comments
Closed

async generator missing unawaited coroutine warning #89091

graingert mannequin opened this issue Aug 16, 2021 · 18 comments
Labels
3.13 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-asyncio

Comments

@graingert
Copy link
Mannequin

graingert mannequin commented Aug 16, 2021

BPO 44928
Nosy @graingert

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 2021-08-16.21:29:12.364>
labels = ['interpreter-core', '3.8', '3.9', '3.10', '3.11', '3.7']
title = 'async generator missing unawaited coroutine warning'
updated_at = <Date 2021-08-16.21:29:12.364>
user = 'https://github.com/graingert'

bugs.python.org fields:

activity = <Date 2021-08-16.21:29:12.364>
actor = 'graingert'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2021-08-16.21:29:12.364>
creator = 'graingert'
dependencies = []
files = []
hgrepos = []
issue_num = 44928
keywords = []
message_count = 1.0
messages = ['399684']
nosy_count = 1.0
nosy_names = ['graingert']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue44928'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8', 'Python 3.9', 'Python 3.10', 'Python 3.11']

Linked PRs

@graingert
Copy link
Mannequin Author

graingert mannequin commented Aug 16, 2021

demo program:

def test_async_fn():
    async def async_fn():
        pass

    async_fn()  # Missing 'await'; produces RuntimeWarning


def test_async_gen_fn():
    async def agen_fn():
        yield

    agen_fn().aclose()  # Missing 'await' but no warning
    agen_fn().asend(None)  # Missing 'await' but no warning

test_async_fn()  # Warning
test_async_gen_fn()  # No warnings

output:

/home/graingert/projects/anyio/foo.py:5: RuntimeWarning: coroutine 'test_async_fn.<locals>.async_fn' was never awaited
  async_fn()
RuntimeWarning: Enable tracemalloc to get the object allocation traceback

expected:

/home/graingert/projects/anyio/foo.py:5: RuntimeWarning: coroutine 'test_async_fn.<locals>.async_fn' was never awaited
  async_fn()
RuntimeWarning: Enable tracemalloc to get the object allocation traceback
/home/graingert/projects/anyio/foo.py:12: RuntimeWarning: coroutine '<async_generator_athrow object at 0xffffffffffff>' was never awaited
  agen_fn().aclose()
RuntimeWarning: Enable tracemalloc to get the object allocation traceback
/home/graingert/projects/anyio/foo.py:13: RuntimeWarning: coroutine '<async_generator_asend object at 0xffffffffffff>' was never awaited
  agen_fn().asend(None)
RuntimeWarning: Enable tracemalloc to get the object allocation traceback

@graingert graingert mannequin added 3.7 (EOL) end of life 3.8 (EOL) end of life 3.9 only security fixes 3.10 only security fixes 3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Aug 16, 2021
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@kumaraditya303 kumaraditya303 added 3.12 only security fixes and removed 3.9 only security fixes 3.8 (EOL) end of life 3.7 (EOL) end of life labels Jul 17, 2022
@kumaraditya303
Copy link
Contributor

This is interesting!

Currently no warning is raised for agen().asend(None) because asend is not implemented as a coroutine but rather asend returns a object which has __await__ so that it can be awaited on. However from a user's perspective a RuntimeWarning makes sense.

@gvanrossum What do you think about raising a RuntimeWarning here?

@gvanrossum
Copy link
Member

Alas, my brain locks up when I start thinking about async generators. Maybe @1st1 can help?

@kumaraditya303
Copy link
Contributor

The implementation is simple, all we need is a finalizer to check if the current ags_state of PyAsyncGenASend is not AWAITABLE_STATE_INIT else raise warning.

@gvanrossum
Copy link
Member

Can you get a 2nd opinion on that from a mailing list or forum? IIRC there's an async-sig still.

@gvanrossum
Copy link
Member

My concern is that this might start warning for code that is totally harmless. Therefore I would definitely not backport any fix to 3.11 or before.

@kumaraditya303
Copy link
Contributor

My concern is that this might start warning for code that is totally harmless.

It is not harmless, the code does nothing if it is not awaited and currently it doesn't even emit RuntimeWarning.

@kumaraditya303
Copy link
Contributor

@graingert Would you post this to discourse as Guido said for discussion?

@graingert
Copy link
Contributor

https://discuss.python.org/t/async-generator-missing-unawaited-coroutine-warning/17563/2 done

Btw being tagged in the Nosy doesn't seem to subscribe me to issues

@njsmith
Copy link
Contributor

njsmith commented Jul 21, 2022

Yeah, this is a pretty simple issue. Conceptually the asend and athrow methods on async generators are just regular async functions, and if they were implemented that way they'd automatically raise the same "forgot to await" warnings that all async functions get. But because they're implemented directly in C, they don't get that infrastructure "for free"; it has to be added manually, and it's one of those minor fiddly details that's easy to miss when you're implementing something complicated like this.

@gvanrossum
Copy link
Member

Okay, I'll unsub this issue and you all can figure it out.

@github-project-automation github-project-automation bot moved this to Todo in asyncio Jan 9, 2023
@kumaraditya303 kumaraditya303 removed 3.11 only security fixes 3.10 only security fixes 3.12 only security fixes labels May 15, 2023
@kumaraditya303
Copy link
Contributor

I have hit this again today, I'll fix it in 3.13.

@markshannon
Copy link
Member

Is this right solution to the problem of "lost" async generators?

Can we pause and ask why this such a problem in the first place?
Perhaps we need better syntax or compiler warnings, instead of adding more band-aids?

Trying to improve the performance of async code is a challenge due to the layers of C code between the sender and the coroutine. It is can be worked around, but it is making more work for us trying to speed up async code.

We added a big speedup for iterating over generators in 3.12. Doing the same for async generators is much harder due to the extra machinery. Adding more machinery is going to make it harder still.

This is a more general problem than this particular issue, but I'd rather not make the situation any worse.

@kumaraditya303
Copy link
Contributor

kumaraditya303 commented May 18, 2023

Is this right solution to the problem of "lost" async generators?

The problem isn't "lost" async generators but its unawaited methods like asend, athrow etc.

Perhaps we need better syntax or compiler warnings, instead of adding more band-aids?

I don't see what better syntax would fix here, it is a user experience thing, and I have hit this issue multiple times.

We added a big speedup for iterating over generators in 3.12. Doing the same for async generators is much harder due to the extra machinery. Adding more machinery is going to make it harder still.

I don't see how adding warning for this would stop any optimization. You probably want to optimize iteration by async for not individual methods like asend. The former is untouched by changes in this issue.

FYI this already exists for coroutines so this isn't a new thing being added.

@markshannon
Copy link
Member

The problem isn't "lost" async generators but its unawaited methods like asend, athrow etc.

Do these methods get called directly often, rather than using await or async for?
What's the use case?

I don't how adding warning for this would stop any optimization. You probably want to optimize iteration by async for not individual methods like asend. The former is untouched by changes in this issue.

Because your PR adds a finalizer, it means that the destruction of the async generator's asend object has side effects, which makes eliding its creation a lot more complex.

If you have a design for an optimization where that isn't a problem, I'd be happy to review it.

@gvanrossum
Copy link
Member

I don't think that eliding the creation and destruction of the asend method object is all that important. This only happens when one uses asend -- this is not a common operation, normally one would use async for, which is unchanged; but asend is provided for completeness. (This is just what @kumaraditya303 said.)

Ditto for athrow, it is probably even more "niche" than asend, except that it is also called by aclose, which might be more common. In fact, it may be getting called by asyncio's default asyncgen hook. The machinery here is hellishly complicated (see PEP 525). But in the asyncio case aclose is only called if the async generator isn't already closed; when an async for loop exits normally (because the generator raises StopAsyncIteration) the generator is already closed, so aclose won't be called.

Note again that asend and athrow are so special, they each have a dedicated object type associated with them (which mostly emulates the coroutine API). Only these two dedicated object types are getting finalizers added by this PR. And instances of these object types only get created when asend or athrow or aclose is called.

All in all, I think there's not much worry in the next few releases about these particular objects getting in the way of optimization; and if they eventually do, well, then we may have to decide to downgrade the user experience slightly, to once again match the experience in 3.12 and before. Until then, I think it's nice to warn users when they forget to call await on these calls (as I did several times while I was investigating how this works).

@kumaraditya303
Copy link
Contributor

Implemented in #104611

@encukou
Copy link
Member

encukou commented Apr 10, 2024

The warning shows up when you break out of an async for that's iterating over an async generator: #117536

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-asyncio
Projects
Status: Done
Development

No branches or pull requests

6 participants