gh-117536: Fix asyncio _asyncgen_finalizer_hook() #117751
Conversation
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.
vstinner
requested review from
1st1,
asvetlov,
gvanrossum,
kumaraditya303 and
willingc
as code owners
|
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 |
|
@kumaraditya303: Would you mind to have a look at this asyncio change? |
| yield 0 | ||
| yield 1 | ||
| finally: | ||
| ns['state'] = 'finalized' |
There was a problem hiding this comment.
This should await something to prove it's getting aclosed properly
| ns['state'] = 'finalized' | |
| await asyncio.sleep(0) | |
| ns['state'] = 'finalized' |
There was a problem hiding this comment.
Right. I added a second test. I prefer to have two tests, since having 'await' also changes the behavior in a subtle way.
| @@ -555,10 +560,22 @@ def _check_default_executor(self): | |||
| if self._executor_shutdown_called: | |||
| raise RuntimeError('Executor shutdown has been called') | |||
|
|
|||
| async def _asyncgen_close(self, agen): | |||
| await agen.aclose() | |||
There was a problem hiding this comment.
this is just side-stepping the actual bug by delaying the creation of the async_generator_aclose object to avoid the incorrect warning
|
sorry I accidentally closed this PR |
|
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 :-) |
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.