Skip to content

Commit c2f4f3c

Browse files
author
Roman Skurikhin
committed
bp-40607: Reraise exception during task cancelation in asyncio.wait_for()
Currently, if asyncio.wait_for() timeout expires, it cancels inner future and then always raises TimeoutError. In case those future is task, it can handle cancelation mannually, and those process can lead to some other exception. Current implementation silently loses thoses exception. To resolve this, wait_for will check was the cancelation successfull or not. In case there was exception, wait_for will reraise it.
1 parent 7c6e970 commit c2f4f3c

File tree

5 files changed

+66
-6
lines changed

5 files changed

+66
-6
lines changed

Doc/library/asyncio-task.rst

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,8 @@ Timeouts
453453
wrap it in :func:`shield`.
454454

455455
The function will wait until the future is actually cancelled,
456-
so the total wait time may exceed the *timeout*.
456+
so the total wait time may exceed the *timeout*. If an exception
457+
happens during cancellation, it is propagated.
457458

458459
If the wait is cancelled, the future *aw* is also cancelled.
459460

Lib/asyncio/tasks.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -496,7 +496,15 @@ async def wait_for(fut, timeout, *, loop=None):
496496
# after wait_for() returns.
497497
# See https://bugs.python.org/issue32751
498498
await _cancel_and_wait(fut, loop=loop)
499-
raise exceptions.TimeoutError()
499+
# In case task cancellation failed with some
500+
# exception, we should re-raise it
501+
# See https://bugs.python.org/issue40607
502+
try:
503+
fut.result()
504+
except exceptions.CancelledError as exc:
505+
raise exceptions.TimeoutError() from exc
506+
else:
507+
raise exceptions.TimeoutError()
500508
finally:
501509
timeout_handle.cancel()
502510

Lib/test/test_asyncio/test_tasks.py

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,12 @@ def __await__(self):
8080
return self
8181

8282

83+
# The following value can be used as a very small timeout:
84+
# it passes check "timeout > 0", but has almost
85+
# no effect on the test performance
86+
_EPSILON = 0.0001
87+
88+
8389
class BaseTaskTests:
8490

8591
Task = None
@@ -877,12 +883,53 @@ async def inner():
877883

878884
inner_task = self.new_task(loop, inner())
879885

880-
with self.assertRaises(asyncio.TimeoutError):
881-
await asyncio.wait_for(inner_task, timeout=0.1)
886+
await asyncio.wait_for(inner_task, timeout=_EPSILON)
882887

883-
self.assertTrue(task_done)
888+
with self.assertRaises(asyncio.TimeoutError) as cm:
889+
loop.run_until_complete(foo())
884890

885-
loop.run_until_complete(foo())
891+
self.assertTrue(task_done)
892+
chained = cm.exception.__context__
893+
self.assertEqual(type(chained), asyncio.CancelledError)
894+
895+
def test_wait_for_reraises_exception_during_cancellation(self):
896+
loop = asyncio.new_event_loop()
897+
self.addCleanup(loop.close)
898+
899+
class FooException(Exception):
900+
pass
901+
902+
async def foo():
903+
async def inner():
904+
try:
905+
await asyncio.sleep(0.2)
906+
finally:
907+
raise FooException
908+
909+
inner_task = self.new_task(loop, inner())
910+
911+
await asyncio.wait_for(inner_task, timeout=_EPSILON)
912+
913+
with self.assertRaises(FooException):
914+
loop.run_until_complete(foo())
915+
916+
def test_wait_for_raises_timeout_error_if_returned_during_cancellation(self):
917+
loop = asyncio.new_event_loop()
918+
self.addCleanup(loop.close)
919+
920+
async def foo():
921+
async def inner():
922+
try:
923+
await asyncio.sleep(0.2)
924+
except asyncio.CancelledError:
925+
return 42
926+
927+
inner_task = self.new_task(loop, inner())
928+
929+
await asyncio.wait_for(inner_task, timeout=_EPSILON)
930+
931+
with self.assertRaises(asyncio.TimeoutError):
932+
loop.run_until_complete(foo())
886933

887934
def test_wait_for_self_cancellation(self):
888935
loop = asyncio.new_event_loop()

Misc/ACKS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1592,6 +1592,7 @@ J. Sipprell
15921592
Ngalim Siregar
15931593
Kragen Sitaker
15941594
Kaartic Sivaraam
1595+
Roman Skurikhin
15951596
Ville Skyttä
15961597
Michael Sloan
15971598
Nick Sloan
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
When cancelling a task due to timeout, :meth:`asyncio.wait_for` will now
2+
propagate the exception if an error happens during cancellation.
3+
Patch by Roman Skurikhin.

0 commit comments

Comments
 (0)