gh-111358: Fix timeout behaviour in BaseEventLoop.shutdown_default_executor (#115622)

ordinary-jamie · web-flow · commit 53d5e6780422 · 2024-02-19T00:01:00.000Z
diff --git a/Lib/asyncio/base_events.py b/Lib/asyncio/base_events.py
@@ -45,6 +45,7 @@
 from . import sslproto
 from . import staggered
 from . import tasks
+from . import timeouts
 from . import transports
 from . import trsock
 from .log import logger
@@ -598,23 +599,24 @@ async def shutdown_default_executor(self, timeout=None):
         thread = threading.Thread(target=self._do_shutdown, args=(future,))
         thread.start()
         try:
-            await future
-        finally:
-            thread.join(timeout)
-
-        if thread.is_alive():
+            async with timeouts.timeout(timeout):
+                await future
+        except TimeoutError:
             warnings.warn("The executor did not finishing joining "
-                             f"its threads within {timeout} seconds.",
-                             RuntimeWarning, stacklevel=2)
+                          f"its threads within {timeout} seconds.",
+                          RuntimeWarning, stacklevel=2)
             self._default_executor.shutdown(wait=False)
+        else:
+            thread.join()
 
     def _do_shutdown(self, future):
         try:
             self._default_executor.shutdown(wait=True)
             if not self.is_closed():
-                self.call_soon_threadsafe(future.set_result, None)
+                self.call_soon_threadsafe(futures._set_result_unless_cancelled,
+                                          future, None)
         except Exception as ex:
-            if not self.is_closed():
+            if not self.is_closed() and not future.cancelled():
                 self.call_soon_threadsafe(future.set_exception, ex)
 
     def _check_running(self):
diff --git a/Lib/test/test_asyncio/test_base_events.py b/Lib/test/test_asyncio/test_base_events.py
@@ -231,6 +231,22 @@ def test_set_default_executor_error(self):
 
         self.assertIsNone(self.loop._default_executor)
 
+    def test_shutdown_default_executor_timeout(self):
+        class DummyExecutor(concurrent.futures.ThreadPoolExecutor):
+            def shutdown(self, wait=True, *, cancel_futures=False):
+                if wait:
+                    time.sleep(0.1)
+
+        self.loop._process_events = mock.Mock()
+        self.loop._write_to_self = mock.Mock()
+        executor = DummyExecutor()
+        self.loop.set_default_executor(executor)
+
+        with self.assertWarnsRegex(RuntimeWarning,
+                                   "The executor did not finishing joining"):
+            self.loop.run_until_complete(
+                self.loop.shutdown_default_executor(timeout=0.01))
+
     def test_call_soon(self):
         def cb():
             pass
diff --git a/Misc/NEWS.d/next/Library/2024-02-18-12-18-12.gh-issue-111358.9yJUMD.rst b/Misc/NEWS.d/next/Library/2024-02-18-12-18-12.gh-issue-111358.9yJUMD.rst
@@ -0,0 +1,2 @@
+Fix a bug in :meth:`asyncio.BaseEventLoop.shutdown_default_executor` to
+ensure the timeout passed to the coroutine behaves as expected.

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	+Fix a bug in :meth:`asyncio.BaseEventLoop.shutdown_default_executor` to
	`2`	`+ensure the timeout passed to the coroutine behaves as expected.`