Issue 42130: AsyncIO's wait_for can hide cancellation in a rare race condition
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) * ![]() |
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:37 | admin | set | github: 86296 |
| 2021-12-02 09:55:13 | dreamsorcerer | set | messages: + msg407521 |
| 2021-12-01 20:29:35 | jamesf | set | nosy:
+ jamesf messages: + msg407489 |
| 2021-09-03 21:58:39 | dreamsorcerer | set | nosy:
+ dreamsorcerer pull_requests: + pull_request26587 |
| 2021-05-13 10:41:25 | ods | set | keywords:
+ patch stage: patch review pull_requests: + pull_request24737 |
| 2021-05-13 08:49:17 | aaliddell | set | nosy:
+ aaliddell |
| 2021-05-09 20:15:37 | nmatravolgyi | set | nosy:
+ nmatravolgyi messages: + msg393336 |
| 2021-04-30 18:58:48 | crowbarz | set | nosy:
+ crowbarz |
| 2020-10-26 06:52:58 | ods | set | nosy:
+ ods |
| 2020-10-25 13:07:28 | tvoinarovskyi | set | messages: + msg379577 |
| 2020-10-24 21:18:28 | chris.jerdonek | set | messages: + msg379541 |
| 2020-10-24 07:53:05 | chris.jerdonek | set | nosy:
+ chris.jerdonek |
| 2020-10-23 16:10:28 | tvoinarovskyi | create | |
