Issue 32751: wait_for(future, ...) should wait for the future (even if a timeout occurs)

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.)

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).

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.

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).

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.

How do you tell the difference between a cancelled task that's about to exit, and one that will never exit?

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().

+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.

https://bugs.python.org/issue33638 is an example of a very tricky bug caused by wait_for.

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?

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.

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 :-).

> 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.

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

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