Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.7k
gh-117536: Fix asyncio _asyncgen_finalizer_hook()#117751
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 ourterms of service andprivacy 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
Make the finalization of asynchronous generators more reliable.Store a strong reference to the asynchronous generator which is beingclosed to make sure that shutdown_asyncgens() can close it even ifasyncio.run() cancels all tasks.
vstinner commentedApr 11, 2024
I'm not sure of my fix, I didn't touch asyncio for a long time. I don't know if The subtle part is in _cancel_all_tasks(loop)loop.run_until_complete(loop.shutdown_asyncgens())loop.run_until_complete(loop.shutdown_default_executor(constants.THREAD_JOIN_TIMEOUT)) First, all tasks are cancelled, including |
vstinner commentedApr 11, 2024
@kumaraditya303: Would you mind to have a look at this asyncio change? |
| yield0 | ||
| yield1 | ||
| finally: | ||
| ns['state']='finalized' |
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.
This should await something to prove it's getting aclosed properly
| ns['state']='finalized' | |
| awaitasyncio.sleep(0) | |
| ns['state']='finalized' |
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.
Right. I added a second test. I prefer to have two tests, since having 'await' also changes the behavior in a subtle way.
| raiseRuntimeError('Executor shutdown has been called') | ||
| asyncdef_asyncgen_close(self,agen): | ||
| awaitagen.aclose() |
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.
this is just side-stepping the actual bug by delaying the creation of the async_generator_aclose object to avoid the incorrect warning
graingert commentedApr 13, 2024
sorry I accidentally closed this PR |
vstinner commentedApr 15, 2024
I'm sorry, I don't understand this code enough to address reviews. I prefer to abandon my PR. If someone wants to take it over, please go ahead :-) |
Uh oh!
There was an error while loading.Please reload this page.
Make the finalization of asynchronous generators more reliable.
Store a strong reference to the asynchronous generator which is being closed to make sure that shutdown_asyncgens() can close it even if asyncio.run() cancels all tasks.