bpo-34037: test_asyncio uses shutdown_default_executor() by vstinner · Pull Request #16284 · python/cpython
@vstinner Apologies for the late response, I've been taking it easier today since I think I've been doing a bit too much recently, trying to avoid the infamous open source burnout you've talked about a few times. (:
Overall, the tests look correct to me. My only concern is a potential timeout issue from the part in close_loop() (lines 511-516 in test_asyncio/utils.py):
def close_loop(loop): if loop._default_executor is not None: if not loop.is_closed(): loop.run_until_complete(loop.shutdown_default_executor()) else: loop._default_executor.shutdown(wait=True)
Since shutdown_default_executor() already attempts to call _default_executor.shutdown(wait=True) within the thread it creates, if there are any issues with joining the threads, presumably calling _default_executor.shutdown(wait=True) again would not resolve the issue and could end up resulting in a complete timeout for test_asyncio if any of the threads in the ThreadPoolExecutor were unable to be joined.
Therefore, I would recommend changing the else block to the following:
else: loop._default_executor.shutdown(wait=False)
This will at least guarantee that the executor will be shutdown completely, regardless of the status of the threads. IMO, a timeout is the worst case scenario, since it gives us far less useful debugging information and makes the issue harder to resolve.
An alternative of this could be:
def close_loop(loop): if loop._default_executor is not None: try: loop.run_until_complete(loop.shutdown_default_executor()) finally: loop._default_executor.shutdown(wait=False)
This would provide further assurance that the executor will be shut down, since loop._default_executor.shutdown(wait=False) would always be called last. But this may come at the cost of slightly reducing performance, since it would be called redundantly in normal scenarios. For that reason, I prefer the first option.