AsyncIO's wait_for can hide cancellation in a rare race condition

[tvoinarovskyi]

BPO	42130
Nosy	@ods, @asvetlov, @cjerdonek, @1st1, @tvoinarovskyi, @Dreamsorcerer, @aaliddell, @crowbarz, @WJCFerguson
PRs	bpo-42130: Fix swallowing of cancellation by wait_for #26097 bpo-42130: Fix for explicit suppressing of cancellations in wait_for() #28149

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2020-10-23.16:10:28.990>
labels = ['type-bug', '3.8', '3.9', '3.10', 'expert-asyncio']
title = "AsyncIO's wait_for can hide cancellation in a rare race condition"
updated_at = <Date 2021-12-02.09:55:13.171>
user = 'https://github.com/tvoinarovskyi'

bugs.python.org fields:

activity = <Date 2021-12-02.09:55:13.171>
actor = 'dreamsorcerer'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['asyncio']
creation = <Date 2020-10-23.16:10:28.990>
creator = 'tvoinarovskyi'
dependencies = []
files = []
hgrepos = []
issue_num = 42130
keywords = ['patch']
message_count = 6.0
messages = ['379454', '379541', '379577', '393336', '407489', '407521']
nosy_count = 10.0
nosy_names = ['ods', 'asvetlov', 'chris.jerdonek', 'yselivanov', 'tvoinarovskyi', 'dreamsorcerer', 'aaliddell', 'nmatravolgyi', 'crowbarz', 'jamesf']
pr_nums = ['26097', '28149']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue42130'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

Linked PRs

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

[tvoinarovskyi]

Hi, during migration to Python 3.8.6 we encountered a behavior change from previous versions: wait_for ignored the request to cancellation and returned instead. After investigation, it seems to be related to the update in bpo-32751 and is only reproduced if the waited task is finished when cancellation of wait_for happens (code mistakes external CancelledError for a timeout).

The following example can reproduce the behavior on both 3.8.6 and 3.9.0 for me:

import asyncio


async def inner():
 return

async def with_for_coro():
 await asyncio.wait_for(inner(), timeout=100)
 await asyncio.sleep(1)
 print('End of with_for_coro. Should not be reached!')

async def main():
 task = asyncio.create_task(with_for_coro())
 await asyncio.sleep(0)
 assert not task.done()
 task.cancel()
 print('Called task.cancel()')
 await task # -> You would expect a CancelledError to be raised.


asyncio.run(main())

Changing the wait time before cancellation slightly will return the correct behavior and CancelledError will be raised.

[cjerdonek]

It looks like bpo-37658 might be the relevant change rather.

Here is the new logic it introduced:

cpython/Lib/asyncio/tasks.py

Lines 483 to 488 in db45529

	if fut.done():
	return fut.result()
	else:
	fut.remove_done_callback(cb)
	fut.cancel()
	raise

(via #21894 )

[tvoinarovskyi]

Hi Chris,
Yes, I do believe that is the respectful change, if we look the CancelledError is not checked to be external or originate from the timer.
Best regards,
Taras Voinarovskyi

[nmatravolgyi]

I've also found this deficiency of asyncio.wait_for by debugging an obscure hang in an application. Back then I've quickly made an issue about it: https://bugs.python.org/issue43389

I've just closed it as duplicate, since this issue covers the same bug and has been around longer.

I'm surprised this issue has not got more attention. This definitely needs a fix. Ignoring a CancellationError is something that is heavily discouraged by the documentation in general. Silently ignoring/eating a cancellation makes this construct unreliable without good workarounds, aside from not using it.

For a specific application, this was so broken, that I had to come up with a fix in the short term. I made an asyncio.wait_for variant available as a library that fixes the problem: https://github.com/Traktormaster/wait-for2

The repository has a detailed description of the issue and the way it fixes it. It also has test cases to assert the behaviour of the builtin and the fixed wait_for construct from the library.

[wjcferguson]

Echoing nmatravolgyi's comments - I got here after hitting this bug and I too am amazed it's been around so long and hasn't been addressed.

It makes cancellation in my application very unreliable, and the reason I need well-controlled cancellation semantics is to allow emergency stop of physical movement.

[Dreamsorcerer]

It has been addressed, PR should be merged this week: #28149

Like most open source projects, it just needed someone to actually propose a fix.