asyncio: Use a canary task to detect end of event loop #3272
Conversation
|
#3173 suggested handling the CancelledError and GeneratorExit branches separately, but simply calling |
|
Hmm, the GeneratorExit code path appears to always log "coroutine was never awaited" too. Maybe we should keep the old destructor and add a thread join there, and leave the canary task to only listen for clean cancellation. |
Async generators have a special shutdown protocol which allows us to detect the end of the event loop and stop our thread. This lets us clean up the thread reliably when the event loop is started/stopped via the tornado IOLoop interfaces (which explicitly know about the selector thread), or when the latest asyncio interfaces are used (asyncio.run or manually calling shutdown_asyncgens). The thread is still leaked when older versions of the asyncio interfaces are used (loop.close *without* shutdown_asyncgens), but I've been unable to find a solution that does not print leak warnings even in the event of a clean shutdown. Use of shutdown_asyncgens is now effectively required for apps combining asyncio and tornado. This is unfortunate since leaking a thread is relatively expensive compared to the usual consequences of failing to call shutdown_asyncgens, but it seems to be the best we can do. Fixes tornadoweb#3173
|
I'm now taking a different approach than the original version of this change: asynchronous generators have a formal, documented shutdown procedure so we can use one to detect the shutdown of the event loop when newer interfaces like asyncio.run are used. This is less fragile than the original attempt to have a task get cancelled. Unfortunately I was never able to find a good GC-based solution for cases when the shutdown_asyncgens method is not called, so we'll still leak the thread in that case. The solution if that becomes a problem will be to add shutdown_asyncgens calls as appropriate (which will also help with other cleanup in async generators). |
Attempt to clean up our selector thread (on windows) when the event loop shuts down without waiting for GC. We can't always do this, but we can at least detect it for asyncio.run. For cases where we do need to rely on GC, we now rely on the GC of the canary Task instead of the AddThreadSelectorEventLoop.
Fixes #3173