From d0a7d405ad738ba7e362e17a4d424f68ec3ac412 Mon Sep 17 00:00:00 2001 From: ordinary-jamie <101677823+ordinary-jamie@users.noreply.github.com> Date: Sun, 18 Feb 2024 12:25:07 +1100 Subject: [PATCH 1/3] Fix timeout behaviour in BaseEventLoop.shutdown_default_executor --- Lib/asyncio/base_events.py | 20 ++++++++++--------- Lib/test/test_asyncio/test_base_events.py | 18 +++++++++++++++++ ...-02-18-12-18-12.gh-issue-111358.9yJUMD.rst | 2 ++ 3 files changed, 31 insertions(+), 9 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2024-02-18-12-18-12.gh-issue-111358.9yJUMD.rst diff --git a/Lib/asyncio/base_events.py b/Lib/asyncio/base_events.py index aadc4f478f8b56..6c5cf28e7c59d4 100644 --- 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 index 82071edb252570..a67620f37d37d5 100644 --- a/Lib/test/test_asyncio/test_base_events.py +++ b/Lib/test/test_asyncio/test_base_events.py @@ -231,6 +231,24 @@ 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 warnings.catch_warnings(record=True) as w: + self.loop.run_until_complete( + self.loop.shutdown_default_executor(timeout=0.01)) + + self.assertEqual(len(w), 1) + self.assertIsInstance(w[0].message, RuntimeWarning) + 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 new file mode 100644 index 00000000000000..2e895f8f181ce7 --- /dev/null +++ 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. From a120c87f12588c2b09a47f21e6c3504300e79cb4 Mon Sep 17 00:00:00 2001 From: ordinary-jamie <101677823+ordinary-jamie@users.noreply.github.com> Date: Mon, 19 Feb 2024 08:06:14 +1100 Subject: [PATCH 2/3] Fix unit test raising warnings --- Lib/test/test_asyncio/test_base_events.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/test_asyncio/test_base_events.py b/Lib/test/test_asyncio/test_base_events.py index a67620f37d37d5..e05a1755b3343e 100644 --- a/Lib/test/test_asyncio/test_base_events.py +++ b/Lib/test/test_asyncio/test_base_events.py @@ -243,6 +243,7 @@ def shutdown(self, wait=True, *, cancel_futures=False): self.loop.set_default_executor(executor) with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("default") self.loop.run_until_complete( self.loop.shutdown_default_executor(timeout=0.01)) From c34ef8a6064ed4b34db3011ee23682ba92ae29c8 Mon Sep 17 00:00:00 2001 From: ordinary-jamie <101677823+ordinary-jamie@users.noreply.github.com> Date: Mon, 19 Feb 2024 10:43:06 +1100 Subject: [PATCH 3/3] Use correct method for testing warnings --- Lib/test/test_asyncio/test_base_events.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_asyncio/test_base_events.py b/Lib/test/test_asyncio/test_base_events.py index e05a1755b3343e..4cd872d3a5b2d8 100644 --- a/Lib/test/test_asyncio/test_base_events.py +++ b/Lib/test/test_asyncio/test_base_events.py @@ -242,14 +242,11 @@ def shutdown(self, wait=True, *, cancel_futures=False): executor = DummyExecutor() self.loop.set_default_executor(executor) - with warnings.catch_warnings(record=True) as w: - warnings.simplefilter("default") + with self.assertWarnsRegex(RuntimeWarning, + "The executor did not finishing joining"): self.loop.run_until_complete( self.loop.shutdown_default_executor(timeout=0.01)) - self.assertEqual(len(w), 1) - self.assertIsInstance(w[0].message, RuntimeWarning) - def test_call_soon(self): def cb(): pass