From 487eac18d204f24ff37b3bebe21f8c717d11cc1e Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Mon, 12 Sep 2022 07:36:17 +0000 Subject: [PATCH 01/28] rewrite wait_for to use timeouts.timeout --- Lib/asyncio/tasks.py | 54 ++++----------------------- Lib/test/test_asyncio/test_waitfor.py | 27 -------------- 2 files changed, 8 insertions(+), 73 deletions(-) diff --git a/Lib/asyncio/tasks.py b/Lib/asyncio/tasks.py index e78719de216fd0..5049435c3cc6c8 100644 --- a/Lib/asyncio/tasks.py +++ b/Lib/asyncio/tasks.py @@ -24,6 +24,7 @@ from . import events from . import exceptions from . import futures +from . import timeouts from .coroutines import _is_coroutine # Helper to generate new task names @@ -439,63 +440,23 @@ async def wait_for(fut, timeout): This function is a coroutine. """ - loop = events.get_running_loop() - if timeout is None: return await fut if timeout <= 0: - fut = ensure_future(fut, loop=loop) + fut = ensure_future(fut) if fut.done(): return fut.result() - await _cancel_and_wait(fut, loop=loop) + await _cancel_and_wait(fut) try: return fut.result() except exceptions.CancelledError as exc: - raise exceptions.TimeoutError() from exc - - waiter = loop.create_future() - timeout_handle = loop.call_later(timeout, _release_waiter, waiter) - cb = functools.partial(_release_waiter, waiter) - - fut = ensure_future(fut, loop=loop) - fut.add_done_callback(cb) - - try: - # wait until the future completes or the timeout - try: - await waiter - except exceptions.CancelledError: - if fut.done(): - return fut.result() - else: - fut.remove_done_callback(cb) - # We must ensure that the task is not running - # after wait_for() returns. - # See https://bugs.python.org/issue32751 - await _cancel_and_wait(fut, loop=loop) - raise - - if fut.done(): - return fut.result() - else: - fut.remove_done_callback(cb) - # We must ensure that the task is not running - # after wait_for() returns. - # See https://bugs.python.org/issue32751 - await _cancel_and_wait(fut, loop=loop) - # In case task cancellation failed with some - # exception, we should re-raise it - # See https://bugs.python.org/issue40607 - try: - return fut.result() - except exceptions.CancelledError as exc: - raise exceptions.TimeoutError() from exc - finally: - timeout_handle.cancel() + raise TimeoutError from exc + async with timeouts.timeout(timeout): + return await fut async def _wait(fs, timeout, return_when, loop): """Internal helper for wait(). @@ -541,9 +502,10 @@ def _on_completion(f): return done, pending -async def _cancel_and_wait(fut, loop): +async def _cancel_and_wait(fut): """Cancel the *fut* future or task and wait until it completes.""" + loop = events.get_running_loop() waiter = loop.create_future() cb = functools.partial(_release_waiter, waiter) fut.add_done_callback(cb) diff --git a/Lib/test/test_asyncio/test_waitfor.py b/Lib/test/test_asyncio/test_waitfor.py index 45498fa097f6bc..0e7d5b6f4e2ee0 100644 --- a/Lib/test/test_asyncio/test_waitfor.py +++ b/Lib/test/test_asyncio/test_waitfor.py @@ -237,33 +237,6 @@ async def inner(): with self.assertRaises(FooException): await foo() - async def test_wait_for_self_cancellation(self): - async def inner(): - try: - await asyncio.sleep(0.3) - except asyncio.CancelledError: - try: - await asyncio.sleep(0.3) - except asyncio.CancelledError: - await asyncio.sleep(0.3) - - return 42 - - inner_task = asyncio.create_task(inner()) - - wait = asyncio.wait_for(inner_task, timeout=0.1) - - # Test that wait_for itself is properly cancellable - # even when the initial task holds up the initial cancellation. - task = asyncio.create_task(wait) - await asyncio.sleep(0.2) - task.cancel() - - with self.assertRaises(asyncio.CancelledError): - await task - - self.assertEqual(await inner_task, 42) - async def _test_cancel_wait_for(self, timeout): loop = asyncio.get_running_loop() From 794cb3adab23005ef2ed9a1fb95d4a1e822a2264 Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Fri, 21 Oct 2022 13:01:29 +0000 Subject: [PATCH 02/28] skip one test --- Lib/test/test_asyncio/test_waitfor.py | 28 +++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/Lib/test/test_asyncio/test_waitfor.py b/Lib/test/test_asyncio/test_waitfor.py index 0e7d5b6f4e2ee0..f0866e409720d3 100644 --- a/Lib/test/test_asyncio/test_waitfor.py +++ b/Lib/test/test_asyncio/test_waitfor.py @@ -237,6 +237,34 @@ async def inner(): with self.assertRaises(FooException): await foo() + @unittest.skip("TODO") + async def test_wait_for_self_cancellation(self): + async def inner(): + try: + await asyncio.sleep(0.3) + except asyncio.CancelledError: + try: + await asyncio.sleep(0.3) + except asyncio.CancelledError: + await asyncio.sleep(0.3) + + return 42 + + inner_task = asyncio.create_task(inner()) + + wait = asyncio.wait_for(inner_task, timeout=0.1) + + # Test that wait_for itself is properly cancellable + # even when the initial task holds up the initial cancellation. + task = asyncio.create_task(wait) + await asyncio.sleep(0.2) + task.cancel() + + with self.assertRaises(asyncio.CancelledError): + await task + + self.assertEqual(await inner_task, 42) + async def _test_cancel_wait_for(self, timeout): loop = asyncio.get_running_loop() From 13ab78fb85bcb4f570a9729a9ee76cd7786ebab7 Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Fri, 21 Oct 2022 13:07:29 +0000 Subject: [PATCH 03/28] fix test as wait_for no longer creates a new task so no sleep is required --- Lib/test/test_asyncio/test_waitfor.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/test/test_asyncio/test_waitfor.py b/Lib/test/test_asyncio/test_waitfor.py index f0866e409720d3..c525696e88931b 100644 --- a/Lib/test/test_asyncio/test_waitfor.py +++ b/Lib/test/test_asyncio/test_waitfor.py @@ -237,7 +237,6 @@ async def inner(): with self.assertRaises(FooException): await foo() - @unittest.skip("TODO") async def test_wait_for_self_cancellation(self): async def inner(): try: @@ -257,7 +256,6 @@ async def inner(): # Test that wait_for itself is properly cancellable # even when the initial task holds up the initial cancellation. task = asyncio.create_task(wait) - await asyncio.sleep(0.2) task.cancel() with self.assertRaises(asyncio.CancelledError): From 1eb005362bc16e1e6cffa41e9197b6020605954a Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Fri, 21 Oct 2022 14:50:57 +0000 Subject: [PATCH 04/28] remove outdated tests --- Lib/test/test_asyncio/test_futures2.py | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/Lib/test/test_asyncio/test_futures2.py b/Lib/test/test_asyncio/test_futures2.py index 9e7a5775a70383..d96f68fc8eb037 100644 --- a/Lib/test/test_asyncio/test_futures2.py +++ b/Lib/test/test_asyncio/test_futures2.py @@ -76,21 +76,6 @@ class CFutureTests(FutureTests, unittest.IsolatedAsyncioTestCase): class PyFutureTests(FutureTests, unittest.IsolatedAsyncioTestCase): cls = tasks._PyTask -class FutureReprTests(unittest.IsolatedAsyncioTestCase): - - async def test_recursive_repr_for_pending_tasks(self): - # The call crashes if the guard for recursive call - # in base_futures:_future_repr_info is absent - # See Also: https://bugs.python.org/issue42183 - - async def func(): - return asyncio.all_tasks() - - # The repr() call should not raise RecursiveError at first. - # The check for returned string is not very reliable but - # exact comparison for the whole string is even weaker. - self.assertIn('...', repr(await asyncio.wait_for(func(), timeout=10))) - if __name__ == '__main__': unittest.main() From c7049965c6724144ae9b5c1e82f930b4fd8a496d Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Sat, 22 Oct 2022 05:27:14 +0000 Subject: [PATCH 05/28] fix future test --- Lib/test/test_asyncio/test_futures2.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Lib/test/test_asyncio/test_futures2.py b/Lib/test/test_asyncio/test_futures2.py index d96f68fc8eb037..bd57c90aa4aaff 100644 --- a/Lib/test/test_asyncio/test_futures2.py +++ b/Lib/test/test_asyncio/test_futures2.py @@ -76,6 +76,21 @@ class CFutureTests(FutureTests, unittest.IsolatedAsyncioTestCase): class PyFutureTests(FutureTests, unittest.IsolatedAsyncioTestCase): cls = tasks._PyTask +class FutureReprTests(unittest.IsolatedAsyncioTestCase): + + async def test_recursive_repr_for_pending_tasks(self): + # The call crashes if the guard for recursive call + # in base_futures:_future_repr_info is absent + # See Also: https://bugs.python.org/issue42183 + + async def func(): + return asyncio.all_tasks() + + # The repr() call should not raise RecursiveError at first. + # The check for returned string is not very reliable but + # exact comparison for the whole string is even weaker. + repr(await asyncio.wait_for(func(), timeout=10)) + if __name__ == '__main__': unittest.main() From bdaed9d0024cc768fe8d2fbe9f4940db1ae28c7e Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Sat, 22 Oct 2022 09:07:59 +0000 Subject: [PATCH 06/28] more tests --- Lib/test/test_asyncio/test_waitfor.py | 35 +++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/Lib/test/test_asyncio/test_waitfor.py b/Lib/test/test_asyncio/test_waitfor.py index c525696e88931b..248db6075eac62 100644 --- a/Lib/test/test_asyncio/test_waitfor.py +++ b/Lib/test/test_asyncio/test_waitfor.py @@ -288,6 +288,41 @@ async def test_cancel_blocking_wait_for(self): async def test_cancel_wait_for(self): await self._test_cancel_wait_for(60.0) + async def test_wait_for_cancel_suppressed(self): + # See https://github.com/python/cpython/issues/84849 + # https://github.com/python/cpython/pull/28149 + + async def return_42(): + try: + await asyncio.sleep(10) + except asyncio.CancelledError: + return 42 + + res = await asyncio.wait_for(return_42(), timeout=0.1) + self.assertEqual(res, 42) + + + async def test_wait_for_issue86296(self): + # See https://github.com/python/cpython/issues/86296 + + async def inner(): + return + + reached_end = False + + async def with_for_coro(): + await asyncio.wait_for(inner(), timeout=100) + await asyncio.sleep(1) + nonlocal reached_end + reached_end = True + + task = asyncio.create_task(with_for_coro()) + await asyncio.sleep(0) + self.assertFalse(task.done()) + task.cancel() + with self.assertRaises(asyncio.CancelledError): + await task + if __name__ == '__main__': unittest.main() From 8ec6b37fd2aa94a4ac5fa9ef7995e436b5d030b0 Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Sat, 22 Oct 2022 09:18:41 +0000 Subject: [PATCH 07/28] fix whitespaces and comment --- Lib/test/test_asyncio/test_waitfor.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_asyncio/test_waitfor.py b/Lib/test/test_asyncio/test_waitfor.py index 248db6075eac62..56b0e352b49609 100644 --- a/Lib/test/test_asyncio/test_waitfor.py +++ b/Lib/test/test_asyncio/test_waitfor.py @@ -290,7 +290,7 @@ async def test_cancel_wait_for(self): async def test_wait_for_cancel_suppressed(self): # See https://github.com/python/cpython/issues/84849 - # https://github.com/python/cpython/pull/28149 + # See https://github.com/python/cpython/pull/28149 async def return_42(): try: @@ -322,6 +322,7 @@ async def with_for_coro(): task.cancel() with self.assertRaises(asyncio.CancelledError): await task + self.assertFalse(reached_end) if __name__ == '__main__': From b711aa15c6c9d5a78d8bb2d38fb15f6a4b3550c9 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sat, 22 Oct 2022 09:26:44 +0000 Subject: [PATCH 08/28] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2022-10-22-09-26-43.gh-issue-96764.Dh9Y5L.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2022-10-22-09-26-43.gh-issue-96764.Dh9Y5L.rst diff --git a/Misc/NEWS.d/next/Library/2022-10-22-09-26-43.gh-issue-96764.Dh9Y5L.rst b/Misc/NEWS.d/next/Library/2022-10-22-09-26-43.gh-issue-96764.Dh9Y5L.rst new file mode 100644 index 00000000000000..80484bbb7d6ef3 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-10-22-09-26-43.gh-issue-96764.Dh9Y5L.rst @@ -0,0 +1 @@ +:func:`asyncio.wait_for` now uses :func:`asyncio.timeout` as it's underlying implementation. Patch by Kumar Aditya. From 69c03b1d9e4d7a848982d14b4368829d82cb782c Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Sat, 22 Oct 2022 15:49:09 +0000 Subject: [PATCH 09/28] remove None special case --- Lib/asyncio/tasks.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Lib/asyncio/tasks.py b/Lib/asyncio/tasks.py index 5049435c3cc6c8..3210889d74a4d4 100644 --- a/Lib/asyncio/tasks.py +++ b/Lib/asyncio/tasks.py @@ -440,10 +440,8 @@ async def wait_for(fut, timeout): This function is a coroutine. """ - if timeout is None: - return await fut - if timeout <= 0: + if timeout is not None and timeout <= 0: fut = ensure_future(fut) if fut.done(): From 92df33b2f6bd8dc2e905a27e65c44407908aff37 Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Sun, 23 Oct 2022 13:10:46 +0530 Subject: [PATCH 10/28] Update Misc/NEWS.d/next/Library/2022-10-22-09-26-43.gh-issue-96764.Dh9Y5L.rst Co-authored-by: Guido van Rossum --- .../next/Library/2022-10-22-09-26-43.gh-issue-96764.Dh9Y5L.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2022-10-22-09-26-43.gh-issue-96764.Dh9Y5L.rst b/Misc/NEWS.d/next/Library/2022-10-22-09-26-43.gh-issue-96764.Dh9Y5L.rst index 80484bbb7d6ef3..a0174291cbc311 100644 --- a/Misc/NEWS.d/next/Library/2022-10-22-09-26-43.gh-issue-96764.Dh9Y5L.rst +++ b/Misc/NEWS.d/next/Library/2022-10-22-09-26-43.gh-issue-96764.Dh9Y5L.rst @@ -1 +1 @@ -:func:`asyncio.wait_for` now uses :func:`asyncio.timeout` as it's underlying implementation. Patch by Kumar Aditya. +:func:`asyncio.wait_for` now uses :func:`asyncio.timeout` as its underlying implementation. Patch by Kumar Aditya. From 0290362ed203e43a541d4a3a4fefe95617b96421 Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Mon, 24 Oct 2022 06:03:13 +0000 Subject: [PATCH 11/28] fix typo --- Lib/test/test_asyncio/test_waitfor.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_asyncio/test_waitfor.py b/Lib/test/test_asyncio/test_waitfor.py index 56b0e352b49609..575d03203cf5d3 100644 --- a/Lib/test/test_asyncio/test_waitfor.py +++ b/Lib/test/test_asyncio/test_waitfor.py @@ -310,13 +310,13 @@ async def inner(): reached_end = False - async def with_for_coro(): + async def wait_for_coro(): await asyncio.wait_for(inner(), timeout=100) await asyncio.sleep(1) nonlocal reached_end reached_end = True - task = asyncio.create_task(with_for_coro()) + task = asyncio.create_task(wait_for_coro()) await asyncio.sleep(0) self.assertFalse(task.done()) task.cancel() From 4d3e9a34571fd1a5c42efc8b55f5656bfa0d8c07 Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Wed, 26 Oct 2022 05:54:51 +0000 Subject: [PATCH 12/28] remove a test --- Lib/test/test_asyncio/test_waitfor.py | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/Lib/test/test_asyncio/test_waitfor.py b/Lib/test/test_asyncio/test_waitfor.py index 575d03203cf5d3..b1366c9a311d65 100644 --- a/Lib/test/test_asyncio/test_waitfor.py +++ b/Lib/test/test_asyncio/test_waitfor.py @@ -237,32 +237,6 @@ async def inner(): with self.assertRaises(FooException): await foo() - async def test_wait_for_self_cancellation(self): - async def inner(): - try: - await asyncio.sleep(0.3) - except asyncio.CancelledError: - try: - await asyncio.sleep(0.3) - except asyncio.CancelledError: - await asyncio.sleep(0.3) - - return 42 - - inner_task = asyncio.create_task(inner()) - - wait = asyncio.wait_for(inner_task, timeout=0.1) - - # Test that wait_for itself is properly cancellable - # even when the initial task holds up the initial cancellation. - task = asyncio.create_task(wait) - task.cancel() - - with self.assertRaises(asyncio.CancelledError): - await task - - self.assertEqual(await inner_task, 42) - async def _test_cancel_wait_for(self, timeout): loop = asyncio.get_running_loop() From 5fece55bb90b3d6e46b5d27d60bb31cbf5e5b9fc Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Wed, 30 Nov 2022 22:00:19 +0530 Subject: [PATCH 13/28] fix comment --- Lib/test/test_asyncio/test_futures2.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Lib/test/test_asyncio/test_futures2.py b/Lib/test/test_asyncio/test_futures2.py index bd57c90aa4aaff..e6f0646cc21be5 100644 --- a/Lib/test/test_asyncio/test_futures2.py +++ b/Lib/test/test_asyncio/test_futures2.py @@ -86,9 +86,7 @@ async def test_recursive_repr_for_pending_tasks(self): async def func(): return asyncio.all_tasks() - # The repr() call should not raise RecursiveError at first. - # The check for returned string is not very reliable but - # exact comparison for the whole string is even weaker. + # The repr() call should not raise RecursionError at first. repr(await asyncio.wait_for(func(), timeout=10)) From 9b0ef93407dd90359c1fc081e472705efc02f0f0 Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Sun, 12 Feb 2023 12:12:03 +0000 Subject: [PATCH 14/28] code review --- Lib/asyncio/tasks.py | 7 +++++++ Lib/test/test_asyncio/test_futures2.py | 3 ++- Lib/test/test_asyncio/test_waitfor.py | 6 ++++-- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Lib/asyncio/tasks.py b/Lib/asyncio/tasks.py index 3210889d74a4d4..07dded34e41cc2 100644 --- a/Lib/asyncio/tasks.py +++ b/Lib/asyncio/tasks.py @@ -438,9 +438,16 @@ async def wait_for(fut, timeout): If the wait is cancelled, the task is also cancelled. + If the task supresses the cancellation and returns a value instead, + that value is returned. + This function is a coroutine. """ + # When timeout <= 0, `asyncio.timeout` raises `TimeoutError` + # whereas `wait_for` disables the timeout so we special case + # it here to preserve the behavior. + if timeout is not None and timeout <= 0: fut = ensure_future(fut) diff --git a/Lib/test/test_asyncio/test_futures2.py b/Lib/test/test_asyncio/test_futures2.py index e6f0646cc21be5..b7cfffb76bd8f1 100644 --- a/Lib/test/test_asyncio/test_futures2.py +++ b/Lib/test/test_asyncio/test_futures2.py @@ -87,7 +87,8 @@ async def func(): return asyncio.all_tasks() # The repr() call should not raise RecursionError at first. - repr(await asyncio.wait_for(func(), timeout=10)) + waiter = await asyncio.wait_for(asyncio.Task(func()),timeout=10) + self.assertIn('...', repr(waiter)) if __name__ == '__main__': diff --git a/Lib/test/test_asyncio/test_waitfor.py b/Lib/test/test_asyncio/test_waitfor.py index b1366c9a311d65..d3adb733bc7aa2 100644 --- a/Lib/test/test_asyncio/test_waitfor.py +++ b/Lib/test/test_asyncio/test_waitfor.py @@ -263,8 +263,10 @@ async def test_cancel_wait_for(self): await self._test_cancel_wait_for(60.0) async def test_wait_for_cancel_suppressed(self): - # See https://github.com/python/cpython/issues/84849 - # See https://github.com/python/cpython/pull/28149 + # GH-86296: Supressing CancelledError is discouraged + # but if a task subpresses CancelledError and returns a value, + # `wait_for` should return the value instead of raising CancelledError. + # This is the same behavior as `asyncio.timeout`. async def return_42(): try: From c98ea70ae9309154a7b2684bce82d807ca0ab8a9 Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Sun, 12 Feb 2023 12:33:37 +0000 Subject: [PATCH 15/28] add shield tests --- Lib/test/test_asyncio/test_waitfor.py | 44 +++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/Lib/test/test_asyncio/test_waitfor.py b/Lib/test/test_asyncio/test_waitfor.py index d3adb733bc7aa2..7903993a5b1982 100644 --- a/Lib/test/test_asyncio/test_waitfor.py +++ b/Lib/test/test_asyncio/test_waitfor.py @@ -301,5 +301,49 @@ async def wait_for_coro(): self.assertFalse(reached_end) +class WaitForShieldTests(unittest.IsolatedAsyncioTestCase): + + async def test_zero_timeout(self): + # With timeout=0 the task gets cancelled immediately + # and the shielded task is not run. + async def coro(): + await asyncio.sleep(0.1) + return 'done' + + task = asyncio.create_task(coro()) + with self.assertRaises(asyncio.TimeoutError): + await asyncio.wait_for(asyncio.shield(task), timeout=0) + + self.assertFalse(task.done()) + self.assertFalse(task.cancelled()) + + async def test_none_timeout(self): + # With timeout=None the timeout is desables so it runs to completion. + async def coro(): + await asyncio.sleep(0.1) + return 'done' + + task = asyncio.create_task(coro()) + await asyncio.wait_for(asyncio.shield(task), timeout=None) + + self.assertTrue(task.done()) + self.assertEqual(await task, "done") + + async def test_shielded_timeout(self): + # `shield` prevents the task from being cancelled. + + async def coro(): + await asyncio.sleep(1) + return 'done' + + task = asyncio.create_task(coro()) + with self.assertRaises(asyncio.TimeoutError): + await asyncio.wait_for(asyncio.shield(task), timeout=0.1) + + self.assertFalse(task.done()) + self.assertFalse(task.cancelled()) + self.assertEqual(await task, "done") + + if __name__ == '__main__': unittest.main() From 8b54ce5aed21a178d4cec218e65055ce530b7de1 Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Sun, 12 Feb 2023 12:37:44 +0000 Subject: [PATCH 16/28] more comments --- Lib/test/test_asyncio/test_waitfor.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_asyncio/test_waitfor.py b/Lib/test/test_asyncio/test_waitfor.py index 7903993a5b1982..99b29a82964807 100644 --- a/Lib/test/test_asyncio/test_waitfor.py +++ b/Lib/test/test_asyncio/test_waitfor.py @@ -279,7 +279,8 @@ async def return_42(): async def test_wait_for_issue86296(self): - # See https://github.com/python/cpython/issues/86296 + # GH-86296: The task should get cancelled + # and not run to completion. async def inner(): return @@ -318,7 +319,7 @@ async def coro(): self.assertFalse(task.cancelled()) async def test_none_timeout(self): - # With timeout=None the timeout is desables so it runs to completion. + # With timeout=None the timeout is disables so it runs to completion. async def coro(): await asyncio.sleep(0.1) return 'done' From 6d392193ad6d6fdb17d4def66961a0b689ccbc19 Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Sun, 12 Feb 2023 12:39:05 +0000 Subject: [PATCH 17/28] whitespace --- Lib/test/test_asyncio/test_waitfor.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Lib/test/test_asyncio/test_waitfor.py b/Lib/test/test_asyncio/test_waitfor.py index 99b29a82964807..de683fbf993b36 100644 --- a/Lib/test/test_asyncio/test_waitfor.py +++ b/Lib/test/test_asyncio/test_waitfor.py @@ -331,8 +331,7 @@ async def coro(): self.assertEqual(await task, "done") async def test_shielded_timeout(self): - # `shield` prevents the task from being cancelled. - + # shield prevents the task from being cancelled. async def coro(): await asyncio.sleep(1) return 'done' From d7efdcb55af61f25a654a6dd009b0ac1f7a8884f Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Sun, 12 Feb 2023 12:40:32 +0000 Subject: [PATCH 18/28] typo --- Lib/test/test_asyncio/test_waitfor.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_asyncio/test_waitfor.py b/Lib/test/test_asyncio/test_waitfor.py index de683fbf993b36..b4b5aa8f56694a 100644 --- a/Lib/test/test_asyncio/test_waitfor.py +++ b/Lib/test/test_asyncio/test_waitfor.py @@ -319,7 +319,8 @@ async def coro(): self.assertFalse(task.cancelled()) async def test_none_timeout(self): - # With timeout=None the timeout is disables so it runs to completion. + # With timeout=None the timeout is disabled so it + # runs till completion. async def coro(): await asyncio.sleep(0.1) return 'done' From 27cfdf94eaa6500f108553d14682337fdb12e153 Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Sun, 12 Feb 2023 12:42:38 +0000 Subject: [PATCH 19/28] formatting --- Lib/test/test_asyncio/test_waitfor.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Lib/test/test_asyncio/test_waitfor.py b/Lib/test/test_asyncio/test_waitfor.py index b4b5aa8f56694a..0fe6f282a3bbee 100644 --- a/Lib/test/test_asyncio/test_waitfor.py +++ b/Lib/test/test_asyncio/test_waitfor.py @@ -279,8 +279,7 @@ async def return_42(): async def test_wait_for_issue86296(self): - # GH-86296: The task should get cancelled - # and not run to completion. + # GH-86296: The task should get cancelled and not run to completion. async def inner(): return From 979f07f4a1bdde059a153ac3d320c92c8211aa62 Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Sun, 12 Feb 2023 13:04:59 +0000 Subject: [PATCH 20/28] improve test --- Lib/test/test_asyncio/test_waitfor.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_asyncio/test_waitfor.py b/Lib/test/test_asyncio/test_waitfor.py index 0fe6f282a3bbee..8b9cfce4322c30 100644 --- a/Lib/test/test_asyncio/test_waitfor.py +++ b/Lib/test/test_asyncio/test_waitfor.py @@ -282,22 +282,24 @@ async def test_wait_for_issue86296(self): # GH-86296: The task should get cancelled and not run to completion. async def inner(): - return + return 'done' + inner_task = asyncio.create_task(inner()) reached_end = False async def wait_for_coro(): - await asyncio.wait_for(inner(), timeout=100) + await asyncio.wait_for(inner_task, timeout=100) await asyncio.sleep(1) nonlocal reached_end reached_end = True task = asyncio.create_task(wait_for_coro()) - await asyncio.sleep(0) self.assertFalse(task.done()) task.cancel() with self.assertRaises(asyncio.CancelledError): await task + self.assertTrue(inner_task.done()) + self.assertEqual(await inner_task, 'done') self.assertFalse(reached_end) From 7a7082c31ea3fb9032d59b2b5b1dc895acb393d5 Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Sun, 12 Feb 2023 13:13:22 +0000 Subject: [PATCH 21/28] more comments --- Lib/asyncio/tasks.py | 63 ++++++++++++++++++++------- Lib/test/test_asyncio/test_waitfor.py | 4 ++ 2 files changed, 52 insertions(+), 15 deletions(-) diff --git a/Lib/asyncio/tasks.py b/Lib/asyncio/tasks.py index 07dded34e41cc2..e78719de216fd0 100644 --- a/Lib/asyncio/tasks.py +++ b/Lib/asyncio/tasks.py @@ -24,7 +24,6 @@ from . import events from . import exceptions from . import futures -from . import timeouts from .coroutines import _is_coroutine # Helper to generate new task names @@ -438,30 +437,65 @@ async def wait_for(fut, timeout): If the wait is cancelled, the task is also cancelled. - If the task supresses the cancellation and returns a value instead, - that value is returned. - This function is a coroutine. """ + loop = events.get_running_loop() - # When timeout <= 0, `asyncio.timeout` raises `TimeoutError` - # whereas `wait_for` disables the timeout so we special case - # it here to preserve the behavior. + if timeout is None: + return await fut - if timeout is not None and timeout <= 0: - fut = ensure_future(fut) + if timeout <= 0: + fut = ensure_future(fut, loop=loop) if fut.done(): return fut.result() - await _cancel_and_wait(fut) + await _cancel_and_wait(fut, loop=loop) try: return fut.result() except exceptions.CancelledError as exc: - raise TimeoutError from exc + raise exceptions.TimeoutError() from exc + + waiter = loop.create_future() + timeout_handle = loop.call_later(timeout, _release_waiter, waiter) + cb = functools.partial(_release_waiter, waiter) + + fut = ensure_future(fut, loop=loop) + fut.add_done_callback(cb) + + try: + # wait until the future completes or the timeout + try: + await waiter + except exceptions.CancelledError: + if fut.done(): + return fut.result() + else: + fut.remove_done_callback(cb) + # We must ensure that the task is not running + # after wait_for() returns. + # See https://bugs.python.org/issue32751 + await _cancel_and_wait(fut, loop=loop) + raise + + if fut.done(): + return fut.result() + else: + fut.remove_done_callback(cb) + # We must ensure that the task is not running + # after wait_for() returns. + # See https://bugs.python.org/issue32751 + await _cancel_and_wait(fut, loop=loop) + # In case task cancellation failed with some + # exception, we should re-raise it + # See https://bugs.python.org/issue40607 + try: + return fut.result() + except exceptions.CancelledError as exc: + raise exceptions.TimeoutError() from exc + finally: + timeout_handle.cancel() - async with timeouts.timeout(timeout): - return await fut async def _wait(fs, timeout, return_when, loop): """Internal helper for wait(). @@ -507,10 +541,9 @@ def _on_completion(f): return done, pending -async def _cancel_and_wait(fut): +async def _cancel_and_wait(fut, loop): """Cancel the *fut* future or task and wait until it completes.""" - loop = events.get_running_loop() waiter = loop.create_future() cb = functools.partial(_release_waiter, waiter) fut.add_done_callback(cb) diff --git a/Lib/test/test_asyncio/test_waitfor.py b/Lib/test/test_asyncio/test_waitfor.py index 8b9cfce4322c30..53d76078331b13 100644 --- a/Lib/test/test_asyncio/test_waitfor.py +++ b/Lib/test/test_asyncio/test_waitfor.py @@ -280,6 +280,8 @@ async def return_42(): async def test_wait_for_issue86296(self): # GH-86296: The task should get cancelled and not run to completion. + # inner completes in one cycle of the event loop so it + # complete before the task is cancelled. async def inner(): return 'done' @@ -295,6 +297,8 @@ async def wait_for_coro(): task = asyncio.create_task(wait_for_coro()) self.assertFalse(task.done()) + # Run the task + await asyncio.sleep(0) task.cancel() with self.assertRaises(asyncio.CancelledError): await task From 1239246fee22c11105c7d2a4c51c52e64d49e6b3 Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Sun, 12 Feb 2023 18:46:45 +0530 Subject: [PATCH 22/28] Update Lib/test/test_asyncio/test_waitfor.py --- Lib/test/test_asyncio/test_waitfor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_asyncio/test_waitfor.py b/Lib/test/test_asyncio/test_waitfor.py index 53d76078331b13..dbdef674c34839 100644 --- a/Lib/test/test_asyncio/test_waitfor.py +++ b/Lib/test/test_asyncio/test_waitfor.py @@ -281,7 +281,7 @@ async def return_42(): async def test_wait_for_issue86296(self): # GH-86296: The task should get cancelled and not run to completion. # inner completes in one cycle of the event loop so it - # complete before the task is cancelled. + # completes before the task is cancelled. async def inner(): return 'done' From f0b54809ab1d4c8251f9e3302f10b7d929850b5a Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Sun, 12 Feb 2023 13:18:28 +0000 Subject: [PATCH 23/28] add back tasks.py changes, somehow got lost --- Lib/asyncio/tasks.py | 63 +++++++++++--------------------------------- 1 file changed, 15 insertions(+), 48 deletions(-) diff --git a/Lib/asyncio/tasks.py b/Lib/asyncio/tasks.py index e78719de216fd0..07dded34e41cc2 100644 --- a/Lib/asyncio/tasks.py +++ b/Lib/asyncio/tasks.py @@ -24,6 +24,7 @@ from . import events from . import exceptions from . import futures +from . import timeouts from .coroutines import _is_coroutine # Helper to generate new task names @@ -437,65 +438,30 @@ async def wait_for(fut, timeout): If the wait is cancelled, the task is also cancelled. + If the task supresses the cancellation and returns a value instead, + that value is returned. + This function is a coroutine. """ - loop = events.get_running_loop() - if timeout is None: - return await fut + # When timeout <= 0, `asyncio.timeout` raises `TimeoutError` + # whereas `wait_for` disables the timeout so we special case + # it here to preserve the behavior. - if timeout <= 0: - fut = ensure_future(fut, loop=loop) + if timeout is not None and timeout <= 0: + fut = ensure_future(fut) if fut.done(): return fut.result() - await _cancel_and_wait(fut, loop=loop) + await _cancel_and_wait(fut) try: return fut.result() except exceptions.CancelledError as exc: - raise exceptions.TimeoutError() from exc - - waiter = loop.create_future() - timeout_handle = loop.call_later(timeout, _release_waiter, waiter) - cb = functools.partial(_release_waiter, waiter) - - fut = ensure_future(fut, loop=loop) - fut.add_done_callback(cb) - - try: - # wait until the future completes or the timeout - try: - await waiter - except exceptions.CancelledError: - if fut.done(): - return fut.result() - else: - fut.remove_done_callback(cb) - # We must ensure that the task is not running - # after wait_for() returns. - # See https://bugs.python.org/issue32751 - await _cancel_and_wait(fut, loop=loop) - raise - - if fut.done(): - return fut.result() - else: - fut.remove_done_callback(cb) - # We must ensure that the task is not running - # after wait_for() returns. - # See https://bugs.python.org/issue32751 - await _cancel_and_wait(fut, loop=loop) - # In case task cancellation failed with some - # exception, we should re-raise it - # See https://bugs.python.org/issue40607 - try: - return fut.result() - except exceptions.CancelledError as exc: - raise exceptions.TimeoutError() from exc - finally: - timeout_handle.cancel() + raise TimeoutError from exc + async with timeouts.timeout(timeout): + return await fut async def _wait(fs, timeout, return_when, loop): """Internal helper for wait(). @@ -541,9 +507,10 @@ def _on_completion(f): return done, pending -async def _cancel_and_wait(fut, loop): +async def _cancel_and_wait(fut): """Cancel the *fut* future or task and wait until it completes.""" + loop = events.get_running_loop() waiter = loop.create_future() cb = functools.partial(_release_waiter, waiter) fut.add_done_callback(cb) From 355f2df0e16e87fbe96840ee58f6cc68fa91a25e Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Thu, 16 Feb 2023 15:24:11 +0000 Subject: [PATCH 24/28] comments++ --- Lib/test/test_asyncio/test_waitfor.py | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_asyncio/test_waitfor.py b/Lib/test/test_asyncio/test_waitfor.py index dbdef674c34839..d920e3ce95b3db 100644 --- a/Lib/test/test_asyncio/test_waitfor.py +++ b/Lib/test/test_asyncio/test_waitfor.py @@ -310,18 +310,30 @@ async def wait_for_coro(): class WaitForShieldTests(unittest.IsolatedAsyncioTestCase): async def test_zero_timeout(self): - # With timeout=0 the task gets cancelled immediately - # and the shielded task is not run. + # `asyncio.shield` creates a new task which wraps the passed in + # awaitable and shields it from cancellation so with timeout=0 + # the task returned by `asyncio.shield` aka shielded_task gets + # cancelled immediately and the task wrapped by is scheduled + # to run. + async def coro(): - await asyncio.sleep(0.1) + await asyncio.sleep(0.01) return 'done' task = asyncio.create_task(coro()) with self.assertRaises(asyncio.TimeoutError): - await asyncio.wait_for(asyncio.shield(task), timeout=0) + shielded_task = asyncio.shield(task) + await asyncio.wait_for(shielded_task, timeout=0) + # Task is running in background self.assertFalse(task.done()) self.assertFalse(task.cancelled()) + self.assertTrue(shielded_task.cancelled()) + + # Wait for the task to complete + await asyncio.sleep(0.1) + self.assertTrue(task.done()) + async def test_none_timeout(self): # With timeout=None the timeout is disabled so it @@ -339,12 +351,12 @@ async def coro(): async def test_shielded_timeout(self): # shield prevents the task from being cancelled. async def coro(): - await asyncio.sleep(1) + await asyncio.sleep(0.1) return 'done' task = asyncio.create_task(coro()) with self.assertRaises(asyncio.TimeoutError): - await asyncio.wait_for(asyncio.shield(task), timeout=0.1) + await asyncio.wait_for(asyncio.shield(task), timeout=0.01) self.assertFalse(task.done()) self.assertFalse(task.cancelled()) From 49d0ee4c8840781491f4daa42b1aceeb3feb8667 Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Thu, 16 Feb 2023 15:25:37 +0000 Subject: [PATCH 25/28] fix typo --- Lib/test/test_asyncio/test_waitfor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_asyncio/test_waitfor.py b/Lib/test/test_asyncio/test_waitfor.py index d920e3ce95b3db..ed80540b2b3852 100644 --- a/Lib/test/test_asyncio/test_waitfor.py +++ b/Lib/test/test_asyncio/test_waitfor.py @@ -313,7 +313,7 @@ async def test_zero_timeout(self): # `asyncio.shield` creates a new task which wraps the passed in # awaitable and shields it from cancellation so with timeout=0 # the task returned by `asyncio.shield` aka shielded_task gets - # cancelled immediately and the task wrapped by is scheduled + # cancelled immediately and the task wrapped by it is scheduled # to run. async def coro(): From 549b03f70bf5c696844e0e2b0acc2e899ce743c3 Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Thu, 16 Feb 2023 15:41:05 +0000 Subject: [PATCH 26/28] remove wrong comment --- Lib/asyncio/tasks.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Lib/asyncio/tasks.py b/Lib/asyncio/tasks.py index 07dded34e41cc2..549c4ed97ae9ee 100644 --- a/Lib/asyncio/tasks.py +++ b/Lib/asyncio/tasks.py @@ -444,10 +444,6 @@ async def wait_for(fut, timeout): This function is a coroutine. """ - # When timeout <= 0, `asyncio.timeout` raises `TimeoutError` - # whereas `wait_for` disables the timeout so we special case - # it here to preserve the behavior. - if timeout is not None and timeout <= 0: fut = ensure_future(fut) From 56e60383397e176d814ed7536114d6e27e969a2a Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Thu, 16 Feb 2023 16:12:20 +0000 Subject: [PATCH 27/28] add code as explanantion --- Lib/asyncio/tasks.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Lib/asyncio/tasks.py b/Lib/asyncio/tasks.py index 549c4ed97ae9ee..4a90ef4f069517 100644 --- a/Lib/asyncio/tasks.py +++ b/Lib/asyncio/tasks.py @@ -443,6 +443,24 @@ async def wait_for(fut, timeout): This function is a coroutine. """ + # The special case for timeout <= 0 is for the follwing case: + # + # async def test_waitfor(): + # func_started = False + # + # async def func(): + # nonlocal func_started + # func_started = True + # + # try: + # await asyncio.wait_for(func(), 0) + # except asyncio.TimeoutError: + # assert not func_started + # else: + # assert False + # + # asyncio.run(test_waitfor()) + if timeout is not None and timeout <= 0: fut = ensure_future(fut) From 1a912f48bb60e9b8c55e1bdf73310a439cb31da2 Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Thu, 16 Feb 2023 21:45:24 +0530 Subject: [PATCH 28/28] Update Lib/asyncio/tasks.py --- Lib/asyncio/tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/asyncio/tasks.py b/Lib/asyncio/tasks.py index 4a90ef4f069517..a2e06d5ef72f42 100644 --- a/Lib/asyncio/tasks.py +++ b/Lib/asyncio/tasks.py @@ -443,7 +443,7 @@ async def wait_for(fut, timeout): This function is a coroutine. """ - # The special case for timeout <= 0 is for the follwing case: + # The special case for timeout <= 0 is for the following case: # # async def test_waitfor(): # func_started = False