Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

wait_for(future, ...) should wait for the future (even if a timeout occurs) #76932

Closed
njsmith opened this issue Feb 2, 2018 · 18 comments
Closed

wait_for(future, ...) should wait for the future (even if a timeout occurs) #76932

njsmith opened this issue Feb 2, 2018 · 18 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes topic-asyncio type-bug An unexpected behavior, bug, or error

Comments

@njsmith
Copy link
Contributor

njsmith commented Feb 2, 2018

BPO 32751
Nosy @giampaolo, @njsmith, @asvetlov, @elprans, @ambv, @1st1, @xgid, @miss-islington
PRs
  • bpo-32751: Wait for task cancellation in asyncio.wait_for() #7216
  • [3.7] bpo-32751: Wait for task cancellation in asyncio.wait_for() (GH-7216) #7223
  • bpo-32751: Wait for task cancel in asyncio.wait_for() when timeout <= 0 #21895
  • [3.9] bpo-32751: Wait for task cancel in asyncio.wait_for() when timeout <= 0 (GH-21895) #21963
  • [3.8] bpo-32751: Wait for task cancel in asyncio.wait_for() when timeout <= 0 (GH-21895) #21967
  • [3.7] bpo-32751: Wait for task cancel in asyncio.wait_for() when timeout <= 0 (GH-21895) #21968
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2018-05-30.01:01:02.210>
created_at = <Date 2018-02-02.20:01:15.318>
labels = ['3.8', 'type-bug', '3.7', 'expert-asyncio']
title = 'wait_for(future, ...) should wait for the future (even if a timeout occurs)'
updated_at = <Date 2020-08-26.20:59:39.330>
user = 'https://github.com/njsmith'

    bugs.python.org fields:

    activity = <Date 2020-08-26.20:59:39.330>
actor = 'yselivanov'
assignee = 'none'
closed = True
closed_date = <Date 2018-05-30.01:01:02.210>
closer = 'yselivanov'
components = ['asyncio']
creation = <Date 2018-02-02.20:01:15.318>
creator = 'njs'
dependencies = []
files = []
hgrepos = []
issue_num = 32751
keywords = ['patch']
message_count = 18.0
messages = ['311509', '311515', '311728', '311764', '311773', '311776', '311777', '318061', '318062', '318064', '318065', '318075', '318076', '318095', '318130', '375938', '375941', '375960']
nosy_count = 8.0
nosy_names = ['giampaolo.rodola', 'njs', 'asvetlov', 'Elvis.Pranskevichus', 'lukasz.langa', 'yselivanov', 'xgdomingo', 'miss-islington']
pr_nums = ['7216', '7223', '21895', '21963', '21967', '21968']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue32751'
versions = ['Python 3.7', 'Python 3.8']
    @njsmith
    Copy link
    Contributor Author

    njsmith commented Feb 2, 2018

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

    @njsmith njsmith added 3.7 (EOL) end of life 3.8 only security fixes topic-asyncio labels Feb 2, 2018
    @1st1
    Copy link
    Member

    1st1 commented Feb 2, 2018

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

    @asvetlov
    Copy link
    Contributor

    asvetlov commented Feb 6, 2018

    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.

    @njsmith
    Copy link
    Contributor Author

    njsmith commented Feb 7, 2018

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

    @asvetlov
    Copy link
    Contributor

    asvetlov commented Feb 7, 2018

    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.

    @njsmith
    Copy link
    Contributor Author

    njsmith commented Feb 7, 2018

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

    @asvetlov
    Copy link
    Contributor

    asvetlov commented Feb 7, 2018

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

    @1st1
    Copy link
    Member

    1st1 commented May 29, 2018

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

    @1st1
    Copy link
    Member

    1st1 commented May 29, 2018

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

    @njsmith
    Copy link
    Contributor Author

    njsmith commented May 29, 2018

    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?

    @1st1
    Copy link
    Member

    1st1 commented May 29, 2018

    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.

    @njsmith
    Copy link
    Contributor Author

    njsmith commented May 29, 2018

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

    @1st1
    Copy link
    Member

    1st1 commented May 29, 2018

    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.

    @1st1
    Copy link
    Member

    1st1 commented May 29, 2018

    New changeset e2b340a by Yury Selivanov (Elvis Pranskevichus) in branch 'master':
    bpo-32751: Wait for task cancellation in asyncio.wait_for() (GH-7216)
    e2b340a

    @miss-islington
    Copy link
    Contributor

    New changeset d8948c5 by Miss Islington (bot) in branch '3.7':
    bpo-32751: Wait for task cancellation in asyncio.wait_for() (GH-7216)
    d8948c5

    @1st1 1st1 closed this as completed May 30, 2018
    @1st1 1st1 added the type-bug An unexpected behavior, bug, or error label May 30, 2018
    @1st1
    Copy link
    Member

    1st1 commented Aug 26, 2020

    New changeset c517fc7 by Elvis Pranskevichus in branch 'master':
    bpo-32751: Wait for task cancel in asyncio.wait_for() when timeout <= 0 (bpo-21895)
    c517fc7

    @ambv
    Copy link
    Contributor

    ambv commented Aug 26, 2020

    New changeset 1036ccb 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)
    1036ccb

    @1st1
    Copy link
    Member

    1st1 commented Aug 26, 2020

    New changeset 57b6988 by Elvis Pranskevichus in branch '3.8':
    [3.8] bpo-32751: Wait for task cancel in asyncio.wait_for() when timeout <= 0 (GH-21895) (bpo-21967)
    57b6988

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.7 (EOL) end of life 3.8 only security fixes topic-asyncio type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    5 participants
    @ambv @1st1 @asvetlov @njsmith @miss-islington