-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
bpo-26552: Fixed case where failing asyncio.ensure_future
did not close the coroutine
#30288
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
Conversation
asyncio.ensure_future
did not close the coroutine
In |
0d0fd3f
to
ea3a9b4
Compare
(Rebased to main) |
*with hope* Does it fix |
I fixed that issue separately. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Kumar, I had procrastinated on reviewing this because there's some complicated stuff going on. I finally found the time and noticed that the bpo issue was written somewhat misleadingly, so I've added a clarification there.
I now finally figured out what the real issue is and how to deal with it, and your code is correct:
- If the argument is a
Future
, nothing more needs to be done - If it is a coroutine (i.e.
async def
), we need to close it ifcreate_task()
raisesRuntimeError
(which it only does for one reason, i.e. if the event loop is closed) - If it is an awaitable (i.e. something with an
__await__()
method), it may not have aclose()
method; the wrapper created by_wrap_awaitable()
doesn't need to be closed
So I'm going to land this now. Thanks for your patience!
@gvanrossum: Please replace |
|
@gvanrossum I think this should be backported to 3.10 and 3.9. |
Yeah, makes sense. Adding the labels now. |
Thanks @kumaraditya303 for the PR, and @gvanrossum for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9. |
Thanks @kumaraditya303 for the PR, and @gvanrossum for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10. |
Sorry, @kumaraditya303 and @gvanrossum, I could not cleanly backport this to |
Sorry @kumaraditya303 and @gvanrossum, I had trouble checking out the |
…lose the coroutine (python#30288)
I am leaving 3.9 as it is too diverged from main. |
|
https://bugs.python.org/issue26552