Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
kumaraditya303 commented Dec 29, 2021 • edited by bedevere-bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-bot
Uh oh!
There was an error while loading. Please reload this page.
asyncio.ensure_future did not close the coroutineUh oh!
There was an error while loading. Please reload this page.
dudmz commented Dec 29, 2021 • edited by gvanrossum
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by gvanrossum
Uh oh!
There was an error while loading. Please reload this page.
In |
0d0fd3f to ea3a9b4Comparekumaraditya303 commented Jan 24, 2022
(Rebased to main) |
arhadthedev commented Jan 24, 2022
*with hope* Does it fix |
kumaraditya303 commented Jan 26, 2022
I fixed that issue separately. |
gvanrossum left a comment
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!
bedevere-bot commented Jan 28, 2022
@gvanrossum: Please replace |
bedevere-bot commented Jan 28, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
|
kumaraditya303 commented Jan 29, 2022
@gvanrossum I think this should be backported to 3.10 and 3.9. |
gvanrossum commented Jan 29, 2022
Yeah, makes sense. Adding the labels now. |
miss-islington commented Jan 29, 2022
Thanks @kumaraditya303 for the PR, and @gvanrossum for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9. |
miss-islington commented Jan 29, 2022
Thanks @kumaraditya303 for the PR, and @gvanrossum for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10. |
miss-islington commented Jan 29, 2022
Sorry, @kumaraditya303 and @gvanrossum, I could not cleanly backport this to |
miss-islington commented Jan 29, 2022
Sorry @kumaraditya303 and @gvanrossum, I had trouble checking out the |
…lose the coroutine (python#30288)
kumaraditya303 commented Jan 29, 2022
I am leaving 3.9 as it is too diverged from main. |
bedevere-bot commented Jan 29, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
|
https://bugs.python.org/issue26552