From efca5907bacb6c59788a3fed5b4d3ec4cd24bcd6 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Thu, 17 Feb 2022 15:13:08 +0200 Subject: [PATCH 1/6] Handle race conditions when multiple timeouts occurs at the same loop iteration or task was cancelled just before timeout occurs --- async_timeout/__init__.py | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/async_timeout/__init__.py b/async_timeout/__init__.py index 4188a98..f71e115 100644 --- a/async_timeout/__init__.py +++ b/async_timeout/__init__.py @@ -113,7 +113,7 @@ def __exit__( exc_val: Optional[BaseException], exc_tb: Optional[TracebackType], ) -> Optional[bool]: - self._do_exit(exc_type) + self._do_exit(exc_type, exc_val) return None async def __aenter__(self) -> "Timeout": @@ -126,7 +126,7 @@ async def __aexit__( exc_val: Optional[BaseException], exc_tb: Optional[TracebackType], ) -> Optional[bool]: - self._do_exit(exc_type) + self._do_exit(exc_type, exc_val) return None @property @@ -206,17 +206,32 @@ def _do_enter(self) -> None: self._state = _State.ENTER self._reschedule() - def _do_exit(self, exc_type: Optional[Type[BaseException]]) -> None: + def _do_exit( + self, + exc_type: Optional[Type[BaseException]], + exc_val: Optional[BaseException], + ) -> None: if exc_type is asyncio.CancelledError and self._state == _State.TIMEOUT: - self._timeout_handler = None - raise asyncio.TimeoutError + skip = False + if sys.version_info >= (3, 9): + # Analyse msg + assert exc_val is not None + if exc_val.args and exc_val.args[0] != id(self): + skip = True + if not skip: + if sys.version_info >= (3, 11): + asyncio.current_task().uncancel() + raise asyncio.TimeoutError # timeout has not expired self._state = _State.EXIT self._reject() return None def _on_timeout(self, task: "asyncio.Task[None]") -> None: - task.cancel() + if sys.version_info >= (3, 9): + task.cancel(id(self)) + else: + task.cancel() self._state = _State.TIMEOUT # drop the reference early self._timeout_handler = None From a484e012b0bea8d99ea8e0e785aa07b0e961d6bb Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Thu, 17 Feb 2022 15:46:56 +0200 Subject: [PATCH 2/6] Add tests --- tests/test_timeout.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/test_timeout.py b/tests/test_timeout.py index d32c5fd..675dab4 100644 --- a/tests/test_timeout.py +++ b/tests/test_timeout.py @@ -361,3 +361,34 @@ async def test_deprecated_with() -> None: with pytest.warns(DeprecationWarning): with timeout(1): await asyncio.sleep(0) + + +@pytest.mark.asyncio +async def test_double_timeouts() -> None: + with pytest.raises(asyncio.TimeoutError): + async with timeout(0.2): + async with timeout(0.1): + await asyncio.sleep(10) + + +@pytest.mark.asyncio +async def test_timeout_with_cancelled_task() -> None: + + event = asyncio.Event() + + async def coro() -> None: + event.wait() # a barrier emulation + async with timeout(0): # on the next loop iteration + await asyncio.sleep(5) + + async def main() -> str: + task = asyncio.create_task(coro()) + await event.wait() # a barrier emulation + task.cancel() + with pytest.raises(asyncio.CancelledError): + await task + return "ok" + + task2 = asyncio.create_task(main()) + event.set() # a barrier emulation + assert "ok" == await task2 From 6216748ad74011188e8f892f9b87801fb934c042 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Thu, 17 Feb 2022 15:48:48 +0200 Subject: [PATCH 3/6] Test on Python 3.11-dev --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 44d2681..52d27b3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -47,7 +47,7 @@ jobs: needs: lint strategy: matrix: - pyver: ["3.7", "3.8", "3.9", "3.10"] + pyver: ["3.7", "3.8", "3.9", "3.10", "3.11-dev"] os: [ubuntu, macos, windows] include: - pyver: pypy-3.8 From 74dbba480a068e0f7755b5419a4b6900b35352fd Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Thu, 17 Feb 2022 15:49:47 +0200 Subject: [PATCH 4/6] Add a note --- async_timeout/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/async_timeout/__init__.py b/async_timeout/__init__.py index f71e115..db037f7 100644 --- a/async_timeout/__init__.py +++ b/async_timeout/__init__.py @@ -228,6 +228,7 @@ def _do_exit( return None def _on_timeout(self, task: "asyncio.Task[None]") -> None: + # Note: the second '.cancel()' call is ignored on Python 3.11 if sys.version_info >= (3, 9): task.cancel(id(self)) else: From 90daa6b90543d25a0106f20f4e641450d3e7e1b1 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Thu, 17 Feb 2022 16:57:04 +0200 Subject: [PATCH 5/6] Make sure that .expired works as expected --- async_timeout/__init__.py | 5 +++-- tests/test_timeout.py | 18 ++++++++++++------ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/async_timeout/__init__.py b/async_timeout/__init__.py index db037f7..f95bb18 100644 --- a/async_timeout/__init__.py +++ b/async_timeout/__init__.py @@ -222,8 +222,9 @@ def _do_exit( if sys.version_info >= (3, 11): asyncio.current_task().uncancel() raise asyncio.TimeoutError - # timeout has not expired - self._state = _State.EXIT + # state is EXIT if not timed out previously + if self._state != _State.TIMEOUT: + self._state = _State.EXIT self._reject() return None diff --git a/tests/test_timeout.py b/tests/test_timeout.py index 675dab4..d62b160 100644 --- a/tests/test_timeout.py +++ b/tests/test_timeout.py @@ -366,10 +366,13 @@ async def test_deprecated_with() -> None: @pytest.mark.asyncio async def test_double_timeouts() -> None: with pytest.raises(asyncio.TimeoutError): - async with timeout(0.2): - async with timeout(0.1): + async with timeout(0.1) as cm1: + async with timeout(0.1) as cm2: await asyncio.sleep(10) + assert cm1.expired + assert cm2.expired + @pytest.mark.asyncio async def test_timeout_with_cancelled_task() -> None: @@ -377,18 +380,21 @@ async def test_timeout_with_cancelled_task() -> None: event = asyncio.Event() async def coro() -> None: - event.wait() # a barrier emulation - async with timeout(0): # on the next loop iteration + event.set() + async with timeout_cm: await asyncio.sleep(5) async def main() -> str: task = asyncio.create_task(coro()) - await event.wait() # a barrier emulation + await event.wait() + loop = asyncio.get_running_loop() + timeout_cm.update(loop.time()) # reschedule to the next loop iteration task.cancel() with pytest.raises(asyncio.CancelledError): await task return "ok" + timeout_cm = timeout(3600) # reschedule just before the usage task2 = asyncio.create_task(main()) - event.set() # a barrier emulation assert "ok" == await task2 + assert timeout_cm.expired From 3af70e039d309e9025b7bff8480fed43682a5205 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Fri, 18 Feb 2022 14:55:59 +0200 Subject: [PATCH 6/6] Fix condition --- async_timeout/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/async_timeout/__init__.py b/async_timeout/__init__.py index f95bb18..37c4971 100644 --- a/async_timeout/__init__.py +++ b/async_timeout/__init__.py @@ -216,7 +216,7 @@ def _do_exit( if sys.version_info >= (3, 9): # Analyse msg assert exc_val is not None - if exc_val.args and exc_val.args[0] != id(self): + if not exc_val.args or exc_val.args[0] != id(self): skip = True if not skip: if sys.version_info >= (3, 11):