| msg311509 - (view) |
Author: Nathaniel Smith (njs) *  |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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 |
|
|
| msg375938 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2020-08-26 16:42 |
| New changeset c517fc712105c8e5930cb42baaebdbe37fc3e15f by Elvis Pranskevichus in branch 'master': bpo-32751: Wait for task cancel in asyncio.wait_for() when timeout <= 0 (#21895) https://github.com/python/cpython/commit/c517fc712105c8e5930cb42baaebdbe37fc3e15f |
|
|
| msg375941 - (view) |
Author: Łukasz Langa (lukasz.langa) * |
Date: 2020-08-26 17:15 |
| New changeset 1036ccb55de4abc70837cb46a72ddbb370b8fc94 by Miss Islington (bot) in branch '3.9': bpo-32751: Wait for task cancel in asyncio.wait_for() when timeout <= 0 (GH-21895) (GH-21963) https://github.com/python/cpython/commit/1036ccb55de4abc70837cb46a72ddbb370b8fc94 |
|
|
| msg375960 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2020-08-26 20:59 |
| New changeset 57b698886b47bb81c782c2ba80a8a72fe66c7aad by Elvis Pranskevichus in branch '3.8': [3.8] bpo-32751: Wait for task cancel in asyncio.wait_for() when timeout <= 0 (GH-21895) (#21967) https://github.com/python/cpython/commit/57b698886b47bb81c782c2ba80a8a72fe66c7aad |
|
|