classification
Title: wait_for(future, ...) should wait for the future (even if a timeout occurs)
Type: behavior Stage: resolved
Components: asyncio Versions: Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: asvetlov, giampaolo.rodola, miss-islington, njs, xgdomingo, yselivanov
Priority: normal Keywords: patch

Created on 2018-02-02 20:01 by njs, last changed 2018-05-30 01:01 by yselivanov. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 7216 merged Elvis.Pranskevichus, 2018-05-29 19:40
PR 7223 merged miss-islington, 2018-05-29 21:32
Messages (15)
msg311509 - (view) Author: Nathaniel Smith (njs) * (Python committer) Date: 2018-02-02 20:01
Currently, if you use asyncio.wait_for(future, timeout=....) and the timeout expires, then it (a) cancels to the future, and then (b) returns. This is fine if the future is a Future, because Future.cancel is synchronous and completes immediately. But if the future is a Task, then Task.cancel merely requests cancellation, and it will complete later (or not). In particular, this means that wait_for(coro, ...) can return with the coroutine still running, which is surprising.

(Originally encountered by Alex Grönholm, who was using code like

async with aclosing(agen):
    await wait_for(agen.asend(...), timeout=...)

and then confused about why the call to agen.aclose was raising an error complaining that agen.asend was still running. Currently this requires an async_generator based async generator to trigger; with a native async generator, the problem is masked by bpo-32526.)
msg311515 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-02-02 21:02
Looks like a bug.  Andrew, if you have time to look at this, please feel free to go ahead; I'm going to be unavailable till Feb 12 (so I can take a look myself after that).
msg311728 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2018-02-06 15:50
Hmm, I don't see an easy way to fix it.
awaiting for cancelled task potentially can wait forever.
Adding another timeout looks too confusing.
Iterating over a couple of loop steps is not reliable: try/finally block in awaited task can perform async IO with unpredictable completion time.
msg311764 - (view) Author: Nathaniel Smith (njs) * (Python committer) Date: 2018-02-07 06:08
If a task refuses to be cancelled, then is waiting for it forever actually wrong? That's the same thing as happens if I do 'task.cancel(); await task', right? Currently wait_for will abandon such a task, but then it's still left running in the background indefinitely (creating a memory leak or worse).
msg311773 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2018-02-07 08:01
Agree.
Should we report about cancelled but still executing tasks?
It would be a nice feature.
I'm talking not about `wait_for` only but task cancellation in general.
msg311776 - (view) Author: Nathaniel Smith (njs) * (Python committer) Date: 2018-02-07 08:46
How do you tell the difference between a cancelled task that's about to exit, and one that will never exit?
msg311777 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2018-02-07 09:08
Theoretically we can start monitoring cancelled tasks and report about them if the task is still not finished, say, in a minute or two.
It is a new feature, sure.

I'm fine with waiting for cancelled task in wait_for().
msg318061 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-05-29 18:10
+1 one to fix this in 3.7, Elvis is working on the patch.  

I don't think we should backport to 3.6 though as it's a behaviour change that people might not expect to get from a bug-fix release.
msg318062 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-05-29 18:22
https://bugs.python.org/issue33638 is an example of a very tricky bug caused by wait_for.
msg318064 - (view) Author: Nathaniel Smith (njs) * (Python committer) Date: 2018-05-29 18:26
Wow, yeah, that is a tricky one.

Didn't Ned say, though, that at this point we should be treating 3.7 like an already-released bugfix-only branch?
msg318065 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-05-29 18:31
Well, we'll have another beta (beta 5) and then a release candidate.  I think it's enough.  I don't feel comfortable with asyncio living with this bug till 3.8.
msg318075 - (view) Author: Nathaniel Smith (njs) * (Python committer) Date: 2018-05-29 19:34
Fair enough. And I can't think of any specific way that fixing this is likely to break anyone, just it's subtle enough that I don't necessarily trust my intuition :-).
msg318076 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-05-29 19:37
> Fair enough. And I can't think of any specific way that fixing this is likely to break anyone, just it's subtle enough that I don't necessarily trust my intuition :-).

In case we find out it doesn't work or causes problems during the beta/rc period, we will simply pull it out.
msg318095 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-05-29 21:31
New changeset e2b340ab4196e1beb902327f503574b5d7369185 by Yury Selivanov (Elvis Pranskevichus) in branch 'master':
bpo-32751: Wait for task cancellation in asyncio.wait_for() (GH-7216)
https://github.com/python/cpython/commit/e2b340ab4196e1beb902327f503574b5d7369185
msg318130 - (view) Author: miss-islington (miss-islington) Date: 2018-05-29 22:37
New changeset d8948c5e09c4a2a818f6f6cfaf8064f2c2138fa5 by Miss Islington (bot) in branch '3.7':
bpo-32751: Wait for task cancellation in asyncio.wait_for() (GH-7216)
https://github.com/python/cpython/commit/d8948c5e09c4a2a818f6f6cfaf8064f2c2138fa5
History
Date User Action Args
2018-05-30 01:01:02yselivanovsetstatus: open -> closed
type: behavior
resolution: fixed
stage: patch review -> resolved
2018-05-29 22:37:09miss-islingtonsetnosy: + miss-islington
messages: + msg318130
2018-05-29 21:32:27miss-islingtonsetpull_requests: + pull_request6854
2018-05-29 21:31:05yselivanovsetmessages: + msg318095
2018-05-29 19:40:57Elvis.Pranskevichussetkeywords: + patch
stage: patch review
pull_requests: + pull_request6848
2018-05-29 19:37:36yselivanovsetmessages: + msg318076
2018-05-29 19:34:54njssetmessages: + msg318075
2018-05-29 18:31:41yselivanovsetmessages: + msg318065
2018-05-29 18:26:02njssetmessages: + msg318064
2018-05-29 18:24:04yselivanovlinkissue33638 superseder
2018-05-29 18:22:29yselivanovsetmessages: + msg318062
2018-05-29 18:10:31yselivanovsetmessages: + msg318061
versions: - Python 3.5, Python 3.6
2018-02-11 02:45:45xgdomingosetnosy: + xgdomingo
2018-02-07 09:08:31asvetlovsetmessages: + msg311777
2018-02-07 08:46:38njssetmessages: + msg311776
2018-02-07 08:01:20asvetlovsetmessages: + msg311773
2018-02-07 06:08:36njssetmessages: + msg311764
2018-02-06 15:50:36asvetlovsetmessages: + msg311728
2018-02-02 21:02:20yselivanovsetmessages: + msg311515
2018-02-02 20:01:15njscreate