bpo-34037: Fix test_asyncio failure and add loop.shutdown_default_executor() by aeros · Pull Request #15735 · python/cpython (original) (raw)
This PR was based heavily upon @vstinner's PR and the suggestions from @asvetlov to improve upon it:
I support the idea but have a question about implementation.
asyncio provides an interface class for loop called AbstractEventLoop.
I believe that the new property should be reflected there.
The next question is: should it be a property or get_wait_executor_on_close() / set_wait_executor_on_close() pair of methods? Asyncio loop is designed to don't use properties but getters / setters.
@vstinner's PR was reverted due to it making too drastic of an API change for 3.8 and the concerns from @asvetlov about adding the property without providing public getter and setter methods.
However, the changes proposed still had significant support, and most importantly, it fixes a bug which has been causing intermittent failures within the CI tests for test_asyncio due to dangling threads:
test_run_in_executor_cancel (test.test_asyncio.test_events.KqueueEventLoopTests) ...
Warning -- threading_cleanup() failed to cleanup 1 threads (count: 1, dangling: 2)
Dangling thread: <Thread(ThreadPoolExecutor-13_0, started daemon 34471091200)>
Dangling thread: <_MainThread(MainThread, started 34393318400)>
Since this is involving changes to the public API, I fully plan on adding the appropriate changes to the documentation. But, I would prefer to wait until I receive feedback from @vstinner and @asvetlov about the implementation itself before doing so since they were both significantly involved in @vstinner's previous PR.
I'll cc the asyncio team as well in case anyone else has feedback to provide.
/cc @python/asyncio