This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: AsyncIO's wait_for can hide cancellation in a rare race condition
Type: behavior Stage: patch review
Components: asyncio Versions: Python 3.10, Python 3.9, Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: aaliddell, asvetlov, chris.jerdonek, crowbarz, dreamsorcerer, jamesf, nmatravolgyi, ods, tvoinarovskyi, yselivanov
Priority: normal Keywords: patch

Created on 2020-10-23 16:10 by tvoinarovskyi, last changed 2022-04-11 14:59 by admin.

Pull Requests
URL Status Linked Edit
PR 26097 open ods, 2021-05-13 10:41
PR 28149 open dreamsorcerer, 2021-09-03 21:58
Messages (6)
msg379454 - (view) Author: Taras Voinarovskyi (tvoinarovskyi) * Date: 2020-10-23 16:10
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.
msg379541 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2020-10-24 21:18
It looks like issue 37658 might be the relevant change rather.

Here is the new logic it introduced: https://github.com/python/cpython/blob/db455296be5f792b8c12b7cd7f3962b52e4f44ee/Lib/asyncio/tasks.py#L483-L488

(via https://github.com/python/cpython/pull/21894 )
msg379577 - (view) Author: Taras Voinarovskyi (tvoinarovskyi) * Date: 2020-10-25 13:07
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
msg393336 - (view) Author: nmatravolgyi (nmatravolgyi) * Date: 2021-05-09 20:15
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.
msg407489 - (view) Author: James Ferguson (jamesf) Date: 2021-12-01 20:29
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.
msg407521 - (view) Author: Sam Bull (dreamsorcerer) * Date: 2021-12-02 09:55
It has been addressed, PR should be merged this week: https://github.com/python/cpython/pull/28149

Like most open source projects, it just needed someone to actually propose a fix.
History
Date User Action Args
2022-04-11 14:59:37adminsetgithub: 86296
2021-12-02 09:55:13dreamsorcerersetmessages: + msg407521
2021-12-01 20:29:35jamesfsetnosy: + jamesf
messages: + msg407489
2021-09-03 21:58:39dreamsorcerersetnosy: + dreamsorcerer
pull_requests: + pull_request26587
2021-05-13 10:41:25odssetkeywords: + patch
stage: patch review
pull_requests: + pull_request24737
2021-05-13 08:49:17aaliddellsetnosy: + aaliddell
2021-05-09 20:15:37nmatravolgyisetnosy: + nmatravolgyi
messages: + msg393336
2021-04-30 18:58:48crowbarzsetnosy: + crowbarz
2020-10-26 06:52:58odssetnosy: + ods
2020-10-25 13:07:28tvoinarovskyisetmessages: + msg379577
2020-10-24 21:18:28chris.jerdoneksetmessages: + msg379541
2020-10-24 07:53:05chris.jerdoneksetnosy: + chris.jerdonek
2020-10-23 16:10:28tvoinarovskyicreate