8000 GH-86296: Fix for asyncio.wait_for() swallowing cancellation, and add tests by twisteroidambassador · Pull Request #98607 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

GH-86296: Fix for asyncio.wait_for() swallowing cancellation, and add tests #98607

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed

GH-86296: Fix for asyncio.wait_for() swallowing cancellation, and add tests #98607

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c1649a9
Add tests for wait_for() behavior when things happen simultaneously.
twisteroidambassador Oct 24, 2022
992d843
Actually use do_cancel argument.
twisteroidambassador Oct 24, 2022
55f5547
Ensure wait_for() does not swallow cancellation.
twisteroidambassador Oct 25, 2022
c834122
Remove tests for simultaneous *timeout* and *inner becoming done*.
twisteroidambassador Oct 26, 2022
7660179
Spec change: when cancelled, always raise CancelledError.
twisteroidambassador Oct 26, 2022
02a624c
📜🤖 Added by blurb_it.
blurb-it[bot] Oct 26, 2022
baf5d1b
Merge branch 'main' into wait_for_specs
kumaraditya303 Dec 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Spec change: when cancelled, always raise CancelledError. 
This means swallowing the inner future's result / exception, if they are set simultaneously with the external cancellation.
  • Loading branch information
@twisteroidambassador
twisteroidambassador committed Oct 26, 2022
commit 7660179b100494d47efb316ca575c0af4ad04f1b
3 changes: 0 additions & 3 deletions Lib/asyncio/tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,9 +474,6 @@ async def wait_for(fut, timeout):
# See https://bugs.python.org/issue32751
fut.remove_done_callback(cb)
await _cancel_and_wait(fut, loop=loop)
exc = fut.exception()
if exc: # If fut.cancelled(), this will raise CancelledError
raise exc
raise

if fut.done():
Expand Down
28 changes: 13 additions & 15 deletions Lib/test/test_asyncio/test_waitfor.py
View file
Open in desktop
9493
Original file line number Diff line number Diff line change
Expand Up @@ -291,15 +291,12 @@ async def test_cancel_wait_for(self):

async def simultaneous_self_cancel_and_inner_result(
self,
do_cancel,
waitfor_timeout,
inner_action,
):
"""Construct scenario where external cancellation and
awaitable becoming done happen simultaneously.
inner_acion is one of 'cancel', 'exception' or 'result'.
if do_cancel, the wait_for() call is cancelled at the same time when
inner_action is executed.
Make sure waitfor_timeout > 0.1. Trying to make it == 0.1 is not
a reliable way to make timeout happen at the same time as the above.
"""
Expand All @@ -320,30 +317,31 @@ async def simultaneous_self_cancel_and_inner_result(
else:
assert inner_action == 'result'
inner.set_result('inner result')
if do_cancel:
waitfor_task.cancel()
return await waitfor_task
waitfor_task.cancel()
with self.assertRaises(asyncio.CancelledError):
return await waitfor_task
# Consume inner's exception, to avoid "Future exception was never
# retrieved" messages
if inner_action == 'exception':
self.assertIsInstance(inner.exception(), RuntimeError)

async def test_simultaneous_self_cancel_and_inner_result(self):
for timeout in (10, None):
with self.subTest(waitfor_timeout=timeout):
with self.assertRaises(asyncio.CancelledError):
await self.simultaneous_self_cancel_and_inner_result(
True, timeout, 'result')
await self.simultaneous_self_cancel_and_inner_result(
timeout, 'result')

async def test_simultaneous_self_cancel_and_inner_exc(self):
for timeout in (10, None):
with self.subTest(waitfor_timeout=timeout):
with self.assertRaises(RuntimeError):
await self.simultaneous_self_cancel_and_inner_result(
True, timeout, 'exception')
await self.simultaneous_self_cancel_and_inner_result(
timeout, 'exception')

async def test_simultaneous_self_cancel_and_inner_cancel(self):
for timeout in (10, None):
with self.subTest(waitfor_timeout=timeout):
with self.assertRaises(asyncio.CancelledError):
await self.simultaneous_self_cancel_and_inner_result(
True, timeout, 'cancel')
await self.simultaneous_self_cancel_and_inner_result(
timeout, 'cancel')


if __name__ == '__main__':
Expand Down
0