Repository navigation
Implement asyncio.timeout() context manager #90927
Copy link
Copy link
Closed
Labels
3.11only security fixesonly security fixesdeferred-blockertopic-asynciotype-featureA feature request or enhancementA feature request or enhancement
Description
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 2022-02-16.17:59:12.940>
labels = ['3.11', 'type-feature', 'expert-asyncio']
title = 'Implement asyncio.timeout() context manager'
updated_at = <Date 2022-03-23.21:50:42.068>
user = 'https://github.com/gvanrossum'bugs.python.org fields:
activity = <Date 2022-03-23.21:50:42.068>
actor = 'asvetlov'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['asyncio']
creation = <Date 2022-02-16.17:59:12.940>
creator = 'gvanrossum'
dependencies = []
files = []
hgrepos = []
issue_num = 46771
keywords = ['patch']
message_count = 76.0
messages = ['413345', '413347', '413349', '413350', '413354', '413356', '413358', '413359', '413360', '413361', '413365', '413366', '413367', '413368', '413369', '413370', '413371', '413372', '413376', '413377', '413387', '413388', '413389', '413402', '413409', '413410', '413411', '413415', '413445', '413460', '413461', '413465', '413466', '413475', '413478', '413480', '413481', '413489', '413491', '413591', '413600', '413601', '413602', '413603', '413604', '413607', '413608', '413610', '413611', '413612', '413613', '413614', '413618', '413619', '413621', '413622', '413623', '413624', '413625', '413631', '413632', '413633', '413760', '413767', '413812', '413820', '413840', '413841', '413874', '414221', '414224', '414853', '414854', '414856', '414857', '414858']
nosy_count = 12.0
nosy_names = ['gvanrossum', 'njs', 'jab', 'asvetlov', 'alex.gronholm', 'chris.jerdonek', 'yselivanov', 'tinchester', 'iritkatriel', 'dreamsorcerer', 'yduprat', 'ajoino']
pr_nums = ['31270', '31394', '31398', '31415', '31434', '31483', '31508', '31513', '31623']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue46771'
versions = ['Python 3.11']Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
3.11only security fixesonly security fixesdeferred-blockertopic-asynciotype-featureA feature request or enhancementA feature request or enhancement
Projects
- StatusShow more project fieldsDone
- StatusShow more project fieldsDone
Activity
gvanrossum commentedon Feb 16, 2022
Now that TaskGroup is merged (see bpo-46752) we might consider adding some form of cancel scopes (another Trio idea).
There's a sensible implementation we could use as a starting point in @asvetlov's async-timeout package (https://github.com/aio-libs/async-timeout).
Tinche commentedon Feb 16, 2022
I've essentially forked
async-timeout(a very good library) into Quattro cancel scopes: https://github.com/Tinche/quattro/blob/main/src/quattro/cancelscope.py.The differences are:
timeoutyou'd usefail_afterormove_on_afterasync with timeout, you use a normal context managerwith fail_after. The Trio folks think this is important (less suspension points, less race conditions) and I agreeupdateandshiftLet me know if this sounds interesting.
Tinche commentedon Feb 16, 2022
Oh, and Trio's
current_effective_deadlineis also in.gvanrossum commentedon Feb 16, 2022
Sure, we should create the best possible solution.
We have no CI in the stdlib that checks type annotations, so those should probably be moved to a stub file in typeshed. (Ditto for asyncio taskgroups.py.)
Using the new .cancelling()/.uncancel() API added to Task you might be able to avoid hacks using the cancel msg (check how it's used in the new TaskGroup).
agronholm commentedon Feb 16, 2022
A brief explanation of cancel scopes for the uninitiated: A cancel scope can enclose just a part of a coroutine function, or an entire group of tasks. They can be nested within each other (by using them as context managers), and marked as shielded from cancellation, which means cancellation won't be propagated (i.e. raised in the coroutine function) from a cancelled outer scope until either the inner scope's shielding is disabled or the inner scope is exited or cancelled directly.
The fundamental problem in implementing these on top of asyncio is that native task cancellation can throw a wrench in these gears. Since a cancel scope works by catching a cancellation error and then (potentially) allowing the coroutine to proceed, it would have to know, when catching a cancellation error, if the cancellation error was targeted at a cancel scope or the task itself. A workaround for this, made possible in Python 3.9, is to (ab)use cancellation messages to include the ID of the target cancel scope. This only solves half of the problem, however. If the task is already pending a cancellation targeted at a cancel scope, the task itself cannot be cancelled anymore since calling cancel() again on the task is a no-op. This would be solved by updating the cancel message on the second call. The docs don't say anything about the behavior on the second call, so it's not strictly speaking a change in documented behavior.
Then, on the subject of level cancellation: level cancellation builds upon cancel scopes and changes cancellation behavior so that whenever a task yields while a cancelled cancel scope is in effect, it gets hit with a CancelledError every time, as opposed to just once in asyncio's "edge" style cancellation. Another very important difference is that with level cancellation, even a task that starts within a cancelled scope gets to run up until the first yield point. This gives it an opportunity to clean up any resources it was given ownership of (a connected socket in a socket server is a common, practical example of this).
This is what the asyncio documentation states about Task.cancel():
"This arranges for a CancelledError exception to be thrown into the wrapped coroutine on the next cycle of the event loop.
The coroutine then has a chance to clean up or even deny the request by suppressing the exception with a try … … except CancelledError … finally block. Therefore, unlike Future.cancel(), Task.cancel() does not guarantee that the Task will be cancelled, although suppressing cancellation completely is not common and is actively discouraged."
This is, however, only true for a task that has started running. A Task that gets cancelled before even entering the coroutine is silently dropped.
As asyncio does not allow for custom task instances without overriding the entire task factory, it leaves libraries like AnyIO some less desirable options for implementing level cancellation:
Having low level machinery for injecting a custom Task instance to the event loop would probably solve this problem.
gvanrossum commentedon Feb 16, 2022
Alex, the goal here is not to replicate every Trio feature or behavior. For example I don't think that asyncio is likely to get level cancellation in 3.11, but we can certainly see if we can do something about some of the edge cases you mention, like the case of a task that is cancelled before it has started running, where you say that the task should be allowed to run until its first await.
It would be nice to have a native asyncio example that demonstrates this, so we have a concrete goal.
I am thinking it is something like this:
This is an interesting edge case and I can see why you'd rather see this run until the
await s.send(data)line. The question is, can we do that without breaking other promises implicit or explicit? (Just because the docs don't mention some behavior that doesn't mean we can change it. We have to consider what happens to actual real world code.)I don't even know if this would be easy to change if we decided it was a good change. Thoughts? (Possibly this discussion belongs in a new issue, since it's not directly related to adding cancel scopes.)
agronholm commentedon Feb 16, 2022
I'm not trying to argue that asyncio should be changed to have level cancellation or even cancel scopes as built-in (at this point), but expanding the low level API to make implementing these features possible in third party libraries without the awkward hacks we have now.
As for async-timeout, it suffers from the same problem as AnyIO and Quattro: that cancellations of the entire task can be inadvertently swallowed by the async context manager in edge cases. I hadn't even thought of the possibility of this happening until one of AnyIO's users reported just such a problem: agronholm/anyio#374
I just couldn't think of any way to correctly support such things without at least _some_ changes to the task cancellation behavior, and allowing .cancel() to update the cancel message seemed like the least invasive option. I'm all ears if someone has a better solution.
gvanrossum commentedon Feb 16, 2022
OK.
Please have a look at how .cancelling()/.uncancel() works (there's an example of it in TaskGroup) to solve that particular problem without using the cancel message.
Suppose you could make (backwards compatible) changes to asyncio. What would you do? 3.11 feature freeze (aka beta 1) is still a few months away (late May) so now's the time to get your wishes in.
agronholm commentedon Feb 16, 2022
Thanks, I will take a look at .uncancel() and .cancelling(). I saw that work happening in my email feed but couldn't figure out right away how it helped, but I will definitely look into the new TaskGroup code to see how it's used there and will get back to you after that.
81 remaining items
gvanrossum commentedon Jul 3, 2022
Sorry to hear it. Get well soon!
gvanrossum commentedon Jul 4, 2022
@pablogsal This is just lacking documentation. Does that qualify for it to be a (deferred) release blocker? The lack of documentation doesn't affect stability!
pablogsal commentedon Jul 5, 2022
Yes, there is no problem with this one. This can be left as a deferred blocker and will not affect any betas or RCs 👍
achimnol commentedon Jul 6, 2022
Though I may have missed critical timing, I'd like to suggest to add ability to customize
msgwhen wecancel()the task, so that user codes can use it to distinguish whether the cancellation is triggered by timeout or other means, as in aio-libs-abandoned/async-timeout#320.Thanks to
cancelling()anduncancel(), we don't have to usemsgas a hack, but I believe it would be useful to leave it a customizable value for the API users since now we can use it for other purposes.It would be nice if we have this in Python 3.12 if it's not possible to add this option in 3.11.
gvanrossum commentedon Jul 6, 2022
Can’t happen in 3.11 due to the feature freeze at 3.11b1, and in fact we decided to deprecate the cancel message. There’s some discussion about it (maybe in this issue, maybe in a linked one).
achimnol commentedon Jul 6, 2022
I totally understand now the 3.11 features are frozen. I just want to note that using cancel messages sometimes greatly helps me to debug and understand what's happening when writing asyncio-based libraries. Of course, it is also easy to abuse. 😉
gvanrossum commentedon Jul 6, 2022
You should probably start a new discussion if you really want to revert the deprecation of the message parameter. E.g. on discuss.python.org.
achimnol commentedon Jul 7, 2022
I've created a thread in discuss.python.org!
Tinche commentedon Jul 9, 2022
I've started on the docs. I'll be using the docs of the
async-timeout,anyio,trioandquattropackages as references, I guess.Where should I put these docs? I'm leaning towards a new section in
asyncio-task.rst.We might want to spin out docs on cancellation to a separate page in the future, since I feel like very few people understand the implications of it, and it's (in my opinion) the second most important asyncio superpower.
gvanrossum commentedon Jul 9, 2022
I agree it belongs in
asyncio-task.rst. It's perhaps most closely related towait_for, so possibly in the same section (on top)? We're not deprecatingwait_for, but we are trying to steer people in the direction of the new context managers.Eventually we may have an "explanation" about cancellation (using the Diataxis term), but that seems a separate doc change.
Tinche commentedon Jul 9, 2022
How about this for a start: Tinche@63221c9
I have a very small section on task cancellation, and the timeout context managers below, but before
wait_for.I feel like a bunch of sections (including the new TaskGroup section) keep mentioning cancellation, so we should at least introduce it fairly quickly.
gvanrossum commentedon Jul 15, 2022
And now that the docs have landed this issue can be laid to rest.