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

@ods

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

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.

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 )

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

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.

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.

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.

can this be fixed by implementing wait_for with asyncio.timeout?:

from . import timeouts

async def wait_for(aw, timeout):
    async with timeouts.timeout(timeout):
        return await aw

I believe using asyncio.shield() can offer a work around to this in at least Python 3.8.6 (I've only tested it in this version). Modifying the test script that reproduces the issue given here as follows (wrapping the coroutine in the wait_for() call in asyncio.shield()):

import asyncio
from typing import List


async def left(tasks: List[asyncio.Task], event: asyncio.Event):
    me = asyncio.current_task()
    assert tasks[0] is me
    await asyncio.sleep(0.1)
    event.set()
    tasks[1].cancel()
    print('cancelled right')


async def right(tasks: List[asyncio.Task], event: asyncio.Event, use_wait_for: bool):
    try:
        me = asyncio.current_task()
        assert tasks[1] is me
        if use_wait_for:
            await asyncio.wait_for(asyncio.shield(event.wait()), 1)
        else:
            await event.wait()
        print('right done, was not cancelled')
    except asyncio.CancelledError:
        print('right got cancelled')
        raise


async def left_right(use_wait_for: bool):
    tasks = []
    event = asyncio.Event()
    tasks.append(asyncio.create_task(left(tasks, event)))
    tasks.append(asyncio.create_task(right(tasks, event, use_wait_for)))
    await asyncio.wait(tasks)


if __name__ == '__main__':
    print('Not using wait_for():')
    asyncio.run(left_right(False))
    print('\nUsing wait_for():')
    asyncio.run(left_right(True))

Gives the desired output:

Not using wait_for():
cancelled right
right got cancelled

Using wait_for():
cancelled right
right got cancelled

This suggests the issue is to do with the exception being swallowed somewhere deep in the call stack.

Add a non-random test to test_stagger.py.

And fixed in 3.12 by gh-96764. (We're not 100.00% sure that the fix doesn't disturb some workaround, so we're not backporting the fix to 3.11 even though that has timeout(), and we would have to devise a totally different fix for 3.10, which we're not inclined to do, sorry. But going forward it should be fixed.

asyncio.wait_for() is too buggy. use util.wait_for2() instead

Add a non-random test to test_stagger.py.

for feedback I just wasted several partial days tracking a production only issue that only happens once a day almost certainly due to this (add logging, narrow down, add more logging..etc...etc). I'd reconsider the decision as it can waste many development hours trying to find what the issue is when the code at hand seems reasonable, until you come to the conclusion that the library must be swallowing the exception. Either that or at a minimum update the docs to say don't use it in python<3.12 or there be dragons.

Phase E: examples, demo #4 (bpo-42130 reproduced), launch draft, publ…

Propagate external cancellation through worker waits on Python 3.10 a…

asyncio.wait_for() is too buggy. use util.wait_for2() instead

fix(ws): Broadcasts erreichen alle Prozesse, nur noch ein Primary Wor…

[Lint] Flag asyncio.wait_for with a pylint check; use the cancel-safe…

Navigation Menu

Repository navigation

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

Linked PRs

18 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

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

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

Description

Linked PRs

Activity

tvoinarovskyi commented on Oct 23, 2020

cjerdonek commented on Oct 24, 2020

tvoinarovskyi commented on Oct 25, 2020

nmatravolgyi commented on May 9, 2021

wjcferguson commented on Dec 1, 2021

Dreamsorcerer commented on Dec 2, 2021

graingert commented on Apr 28, 2022

18 remaining items

laker-93 commented on Oct 28, 2022

gvanrossum commented on May 24, 2023

thehesiod commented on Dec 13, 2024

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions