Skip to content

asyncio.wait_for: process future result produced during cancelation #84849

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.

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

Closed
romasku mannequin opened this issue May 18, 2020 · 6 comments
Closed

asyncio.wait_for: process future result produced during cancelation #84849

romasku mannequin opened this issue May 18, 2020 · 6 comments
Labels
topic-asyncio type-feature A feature request or enhancement

Comments

@romasku
Copy link
Mannequin

romasku mannequin commented May 18, 2020

BPO 40672
Nosy @asvetlov, @cjerdonek, @1st1, @romasku

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 = None
created_at = <Date 2020-05-18.17:23:20.075>
labels = ['type-feature', '3.9', 'expert-asyncio']
title = 'asyncio.wait_for: process future result produced during cancelation'
updated_at = <Date 2020-05-18.22:51:20.953>
user = 'https://github.com/romasku'

bugs.python.org fields:

activity = <Date 2020-05-18.22:51:20.953>
actor = 'chris.jerdonek'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['asyncio']
creation = <Date 2020-05-18.17:23:20.075>
creator = 'Roman Skurikhin'
dependencies = []
files = []
hgrepos = []
issue_num = 40672
keywords = []
message_count = 3.0
messages = ['369278', '369285', '369308']
nosy_count = 4.0
nosy_names = ['asvetlov', 'chris.jerdonek', 'yselivanov', 'Roman Skurikhin']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue40672'
versions = ['Python 3.9']

@romasku
Copy link
Mannequin Author

romasku mannequin commented May 18, 2020

In https://bugs.python.org/issue40607 asyncio.wait_for behavior was changed so it propagates exceptions that happened during cancellation. But it still raises TimeoutError if cancelation ends with some value being returned. In the following example value 42 is lost:

import asyncio


async def return_42_on_cancel():
    try:
        await asyncio.sleep(20)
    except asyncio.CancelledError:
        return 42  # `return` is useless in this block.


async def main():
    try:
        await asyncio.wait_for(return_42_on_cancel(), timeout=1)
    except asyncio.TimeoutError:
        print('Timeout')

asyncio.run(main())

I think it's better to either:

  1. Return that value from asyncio.wait_for.

The motivation here is that if the task returns something, we shouldn't conceal it. I also searched through GitHub and found some places where others catch CancelledError and return value (https://github.com/grpc/grpc/blob/44fb37c99f2853cc23f04fba15468980d9e28e41/src/python/grpcio/grpc/experimental/aio/_interceptor.py#L328).

It can also be used with some coroutines developed to be wrapped with wait_for, for example suppose the following equation solving function:

async def solve_iteratively(initial_x, next_approximation):
    result = initial_x
    try:
        while True:
            result = next_approximation(result)
            await asyncio.sleep(0)
    except asyncio.CancelledError:
        return result

It allows us to control its execution time using asyncio.wait_for.

  1. Add some warning about the value is thrown away (in debug mode) and document it somewhere.

===

I am a newbie here, so sorry if it is wrong to create such "proposal" issues.

@romasku romasku mannequin added 3.9 only security fixes topic-asyncio type-feature A feature request or enhancement labels May 18, 2020
@1st1
Copy link
Member

1st1 commented May 18, 2020

  1. Add some warning about the value is thrown away (in debug mode) and document it somewhere.

The documentation update is definitely something that needs to be done in 3.9. Want to submit a PR?

We can also issue a warning in asyncio debug mode.

I'm really not sure about "1) Return that value from asyncio.wait_for.".

I am a newbie here, so sorry if it is wrong to create such "proposal" issues.

The issue is clear, thanks for submitting it! Keep up the great work.

@cjerdonek
Copy link
Member

Regarding the documentation, I'm not sure we _need_ to say what happens in this edge case for 3.9. It was already unspecified before 3.9, so we're not any worse off. (The change in bpo-40607 was, however, documented.)

I'd rather come to agreement on (1) first. Because if we document it now, then it could be interpreted as defined behavior and so harder to change later due to backwards compat guarantees.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@ezio-melotti ezio-melotti moved this to Todo in asyncio Jul 17, 2022
@ezio-melotti ezio-melotti moved this from Todo to In Progress in asyncio Jul 17, 2022
@ezio-melotti ezio-melotti moved this from In Progress to Todo in asyncio Jul 17, 2022
@kumaraditya303
Copy link
Contributor

kumaraditya303 commented Oct 22, 2022

Duplicate of #86296

@kumaraditya303 kumaraditya303 marked this as a duplicate of #84849 Oct 22, 2022
@kumaraditya303 kumaraditya303 closed this as not planned Won't fix, can't repro, duplicate, stale Oct 22, 2022
Repository owner moved this from Todo to Done in asyncio Oct 22, 2022
@kumaraditya303 kumaraditya303 marked this as a duplicate of #26097 Oct 22, 2022
@gvanrossum
Copy link
Member

Duplicate of #86296

@gvanrossum gvanrossum marked this as a duplicate of #86296 Oct 22, 2022
@gvanrossum gvanrossum reopened this Oct 22, 2022
Repository owner moved this from Done to In Progress in asyncio Oct 22, 2022
@gvanrossum gvanrossum closed this as not planned Won't fix, can't repro, duplicate, stale Oct 22, 2022
Repository owner moved this from In Progress to Done in asyncio Oct 22, 2022
@gvanrossum
Copy link
Member

(And FWIW I think there we're planning to propagate CancelledError in that case.)

@romasku

@kumaraditya303 kumaraditya303 removed the 3.9 only security fixes label Oct 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-asyncio type-feature A feature request or enhancement
Projects
Status: Done
Development

No branches or pull requests

4 participants