From 019ed9d8810c374630e7adc9221c7f8e9377fdfc Mon Sep 17 00:00:00 2001 From: Sam Bull Date: Sun, 24 Oct 2021 16:40:42 +0100 Subject: [PATCH 1/6] Actually fix issue37658 --- Lib/asyncio/tasks.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/asyncio/tasks.py b/Lib/asyncio/tasks.py index 9a9d0d6e3cc269..25ec66b269e730 100644 --- a/Lib/asyncio/tasks.py +++ b/Lib/asyncio/tasks.py @@ -415,7 +415,7 @@ async def wait_for(fut, timeout): await _cancel_and_wait(fut, loop=loop) try: - fut.result() + return fut.result() except exceptions.CancelledError as exc: raise exceptions.TimeoutError() from exc else: @@ -455,7 +455,7 @@ async def wait_for(fut, timeout): # exception, we should re-raise it # See https://bugs.python.org/issue40607 try: - fut.result() + return fut.result() except exceptions.CancelledError as exc: raise exceptions.TimeoutError() from exc else: From ffe5106980d3164ce8cdb07c4ce736fe26d883d5 Mon Sep 17 00:00:00 2001 From: Sam Bull Date: Sun, 24 Oct 2021 16:45:01 +0100 Subject: [PATCH 2/6] Correct test --- Lib/test/test_asyncio/test_tasks.py | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/Lib/test/test_asyncio/test_tasks.py b/Lib/test/test_asyncio/test_tasks.py index 362fbf8df08ca1..182a78da608756 100644 --- a/Lib/test/test_asyncio/test_tasks.py +++ b/Lib/test/test_asyncio/test_tasks.py @@ -1009,20 +1009,15 @@ def gen(): self.assertEqual(res, "ok") def test_wait_for_cancellation_race_condition(self): - def gen(): - yield 0.1 - yield 0.1 - yield 0.1 - yield 0.1 + async def inner(): + await asyncio.wait_for(asyncio.sleep(1), timeout=2) + return 1 - loop = self.new_test_loop(gen) + async def main(): + result = await asyncio.wait_for(inner(), timeout=1) + assert result == 1 - fut = self.new_future(loop) - loop.call_later(0.1, fut.set_result, "ok") - task = loop.create_task(asyncio.wait_for(fut, timeout=1)) - loop.call_later(0.1, task.cancel) - res = loop.run_until_complete(task) - self.assertEqual(res, "ok") + asyncio.run(main()) def test_wait_for_waits_for_task_cancellation(self): loop = asyncio.new_event_loop() From 722fe3a9ed231b4e0e9dfad484f7760b6d2ad0c8 Mon Sep 17 00:00:00 2001 From: Sam Bull Date: Sun, 24 Oct 2021 17:08:08 +0100 Subject: [PATCH 3/6] Update test_tasks.py --- Lib/test/test_asyncio/test_tasks.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/Lib/test/test_asyncio/test_tasks.py b/Lib/test/test_asyncio/test_tasks.py index 182a78da608756..459990ab76718d 100644 --- a/Lib/test/test_asyncio/test_tasks.py +++ b/Lib/test/test_asyncio/test_tasks.py @@ -1096,24 +1096,6 @@ async def inner(): with self.assertRaises(FooException): loop.run_until_complete(foo()) - def test_wait_for_raises_timeout_error_if_returned_during_cancellation(self): - loop = asyncio.new_event_loop() - self.addCleanup(loop.close) - - async def foo(): - async def inner(): - try: - await asyncio.sleep(0.2) - except asyncio.CancelledError: - return 42 - - inner_task = self.new_task(loop, inner()) - - await asyncio.wait_for(inner_task, timeout=_EPSILON) - - with self.assertRaises(asyncio.TimeoutError): - loop.run_until_complete(foo()) - def test_wait_for_self_cancellation(self): loop = asyncio.new_event_loop() self.addCleanup(loop.close) From 54d0118c546dff98c75b0cec2cff8b295f2c922c Mon Sep 17 00:00:00 2001 From: Sam Bull Date: Sun, 24 Oct 2021 17:12:55 +0100 Subject: [PATCH 4/6] Remove redundant else condition. --- Lib/asyncio/tasks.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Lib/asyncio/tasks.py b/Lib/asyncio/tasks.py index 25ec66b269e730..53eef84427be10 100644 --- a/Lib/asyncio/tasks.py +++ b/Lib/asyncio/tasks.py @@ -418,8 +418,6 @@ async def wait_for(fut, timeout): return fut.result() except exceptions.CancelledError as exc: raise exceptions.TimeoutError() from exc - else: - raise exceptions.TimeoutError() waiter = loop.create_future() timeout_handle = loop.call_later(timeout, _release_waiter, waiter) @@ -458,8 +456,6 @@ async def wait_for(fut, timeout): return fut.result() except exceptions.CancelledError as exc: raise exceptions.TimeoutError() from exc - else: - raise exceptions.TimeoutError() finally: timeout_handle.cancel() From 169294e95f8f373317d048bcd89ab3516352314f Mon Sep 17 00:00:00 2001 From: Sam Bull Date: Mon, 25 Oct 2021 20:33:21 +0100 Subject: [PATCH 5/6] Avoid timing issues with test. --- Lib/test/test_asyncio/test_tasks.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_asyncio/test_tasks.py b/Lib/test/test_asyncio/test_tasks.py index 459990ab76718d..a88cb89e4e6eef 100644 --- a/Lib/test/test_asyncio/test_tasks.py +++ b/Lib/test/test_asyncio/test_tasks.py @@ -1010,11 +1010,12 @@ def gen(): def test_wait_for_cancellation_race_condition(self): async def inner(): - await asyncio.wait_for(asyncio.sleep(1), timeout=2) + with contextlib.suppress(asyncio.CancelledError): + await asyncio.sleep(1) return 1 async def main(): - result = await asyncio.wait_for(inner(), timeout=1) + result = await asyncio.wait_for(inner(), timeout=.01) assert result == 1 asyncio.run(main()) From 5cfc8b7c1e691356d0da6668c622e2b9f17eb7bd Mon Sep 17 00:00:00 2001 From: Sam Bull Date: Sun, 28 Nov 2021 15:32:17 +0000 Subject: [PATCH 6/6] Create 2021-11-28-15-30-34.bpo-37658.8Hno7d.rst --- .../next/Library/2021-11-28-15-30-34.bpo-37658.8Hno7d.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2021-11-28-15-30-34.bpo-37658.8Hno7d.rst diff --git a/Misc/NEWS.d/next/Library/2021-11-28-15-30-34.bpo-37658.8Hno7d.rst b/Misc/NEWS.d/next/Library/2021-11-28-15-30-34.bpo-37658.8Hno7d.rst new file mode 100644 index 00000000000000..97d1e961ac2be6 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-11-28-15-30-34.bpo-37658.8Hno7d.rst @@ -0,0 +1,3 @@ +Fix issue when on certain conditions ``asyncio.wait_for()`` may allow a +coroutine to complete successfully, but fail to return the result, +potentially causing memory leaks or other issues.