[issue46771] Add some form of cancel scopes

Alex Grönholm added the comment: Yeah, I'm still interested. I'll create a new BPO when I have something. -- ___ Python tracker ___

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: Good think I forgot to close the issue. ;-) -- ___ Python tracker ___ ___ Python-bugs-list

[issue46771] Add some form of cancel scopes

Andrew Svetlov added the comment: The implementation has landed, docs are still required. -- ___ Python tracker ___ ___

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: I'm closing this, the asyncio.timeout() context manager has been merged. Thanks Andrew! @agronholm you said you were interested in tweaking the cancellation behavior some more. If you're still interested, let's discuss that in a separate bpo (please

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: New changeset f537b2a4fb86445ee3bd6ca7f10bc9d3a9f37da5 by Andrew Svetlov in branch 'main': bpo-46771: Implement asyncio context managers for handling timeouts (GH-31394) https://github.com/python/cpython/commit/f537b2a4fb86445ee3bd6ca7f10bc9d3a9f37da5

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: New changeset 7d611b4cabaf7925f5f94daddf711d54aeae2cf9 by Guido van Rossum in branch 'main': bpo-46771: Remove two controversial lines from Task.cancel() (GH-31623) https://github.com/python/cpython/commit/7d611b4cabaf7925f5f94daddf711d54aeae2cf9

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: Everyone, I've sent a PR (which I expect will make it into alpha 6) that restores the old cancel() semantics. This should make Tin happy, but I think we'll still have to have a longer discussion about the downsides.

[issue46771] Add some form of cancel scopes

Change by Guido van Rossum : -- pull_requests: +29748 pull_request: https://github.com/python/cpython/pull/31623 ___ Python tracker ___

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: New changeset 7fce1063b6e5a366f8504e039a8ccdd6944625cd by Tin Tvrtković in branch 'main': bpo-46771: Implement task cancel requests counter (GH-31513) https://github.com/python/cpython/commit/7fce1063b6e5a366f8504e039a8ccdd6944625cd --

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: I will now merge GH-31513 (cancel counts). Once that's in you can merge main into your timeout PR (GH-31394) and then that can land soon (I'd like to review it once more). -- ___ Python tracker

[issue46771] Add some form of cancel scopes

Andrew Svetlov added the comment: Clear, thanks! -- ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue46771] Add some form of cancel scopes

Change by Yves Duprat : -- nosy: +yduprat ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: To make this cleanly interact with timeout, TaskGroup etc., the CancelOnEvent class should have a "did-I-cancel" flag which is set in the _cancel_on_event() callback. Then if that flag is set it should call .uncancel(), and if that returns a value > 0, it

[issue46771] Add some form of cancel scopes

Andrew Svetlov added the comment: I have no good simple real-case scenario, sorry. There is a demonstration of my thoughts. Suppose we have a custom context manager that behaves similar to timeout() but is controlled not by timer but external event source (it could be an invalidation

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: > If some code is used together with timeout() and this code calls > `.cancel()` but forgot about `.uncancel()` in try/except/finally -- > timeout() never raises TimeoutError. Could you show an example? I'm not sure from this description who cancels whom

[issue46771] Add some form of cancel scopes

Andrew Svetlov added the comment: If some code is used together with timeout() and this code calls `.cancel()` but forgot about `.uncancel()` in try/except/finally -- timeout() never raises TimeoutError. Should we care? The missing `.uncancel()` call is hard to detect by the runtime and

[issue46771] Add some form of cancel scopes

Change by Tin Tvrtković : -- pull_requests: +29640 pull_request: https://github.com/python/cpython/pull/31513 ___ Python tracker ___

[issue46771] Add some form of cancel scopes

Change by Tin Tvrtković : -- pull_requests: +29635 pull_request: https://github.com/python/cpython/pull/31508 ___ Python tracker ___

[issue46771] Add some form of cancel scopes

Change by Tin Tvrtković : -- pull_requests: +29613 pull_request: https://github.com/python/cpython/pull/31483 ___ Python tracker ___

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: PS. The cancel count can work whether or not cancel() returns without setting must-cancel (or cancelling the underlying future/task) when there's already a cancellation in progress. Other things may be different though. We have to look into this

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: [Sam] > It will probably work in this case. But, what about more complex cases? If > there are 2 different timeouts and the possibility of a user cancellation, > and we have a count of 2 cancellations, then what happened? 2 timeouts, or 1 > timeout and

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: [Alex] > The current backward incompatible changes in cancellation behavior are > already causing 10 tests to fail in the AnyIO test suite. I'm trying to find > an alternate solution that does not break anything. Are you sure that the tests aren't

[issue46771] Add some form of cancel scopes

Sam Bull added the comment: > We could store the nonces in a single CancelledError instead. Indeed, my initial proposal was exactly that, but after learning about ExceptionGroup, I thought that was a cleaner approach. -- ___ Python tracker

[issue46771] Add some form of cancel scopes

Alex Grönholm added the comment: > I was under the impression that ExceptionGroup was somewhat backwards > compatible, in that you could still use `except CancelledError:` and it would > catch all the errors in the group. But, maybe I'm wrong, I've not seen the > documentation for the final

[issue46771] Add some form of cancel scopes

Sam Bull added the comment: > Previously, when the task was cancelled twice, only one CancelledError was > raised. Now it would raise a BaseExceptionGroup instead. I was under the impression that ExceptionGroup was somewhat backwards compatible, in that you could still use `except

[issue46771] Add some form of cancel scopes

Alex Grönholm added the comment: > But, if we are using nonces on the CancelledError to keep track, then only 1 > context manager will know if it was themselves or not. This is exactly why > I'm proposing to use multiple CancelledErrors, so that every nonce is passed > to the handling code.

[issue46771] Add some form of cancel scopes

Sam Bull added the comment: Actually, in your counter proposal, you say: > Such context managers should still keep track of whether they cancelled the > task themselves or not, and if they did, they should always call t.uncancel(). But, if we are using nonces on the CancelledError to keep

[issue46771] Add some form of cancel scopes

Alex Grönholm added the comment: > Propagating an ExceptionGroup where every exception can be inspected to see > if it was caused by this code or not still seems like the safe option to me > (and obviously still has the cancel count implicitly). Note that this, too, causes backwards

[issue46771] Add some form of cancel scopes

Sam Bull added the comment: > This should be solved when using the cancel count -- the explicit cancel > bumps the cancel count so the cancel scope (i.e. timeout()) will not raise > TimeoutError. It will probably work in this case. But, what about more complex cases? If there are 2

[issue46771] Add some form of cancel scopes

Change by Tin Tvrtković : -- pull_requests: +29584 pull_request: https://github.com/python/cpython/pull/31434 ___ Python tracker ___

[issue46771] Add some form of cancel scopes

Tin Tvrtković added the comment: @Alex you can follow along here: https://github.com/python/cpython/pull/31394 With the cancel_counter approach, a context manager knows whether to handle or propagate the exception by examining its own local state and the remaining counter on the task. If

[issue46771] Add some form of cancel scopes

Andrew Svetlov added the comment: > This should be solved when using the cancel count -- the explicit cancel > bumps the cancel count so the cancel scope (i.e. timeout()) will not raise > TimeoutError. Hmmm. Interesting! Timeouts are not the single primitive that should care about the

[issue46771] Add some form of cancel scopes

Alex Grönholm added the comment: > It looks more complicated -- the extra parameter needs to be passed around > via the task and then into the CancelledError exception. It reduces overall complexity by making uncancellation unnecessary and restoring backwards compatibility. > What use case

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: (Sam Bull) > To expand on this point, I've been looking at solving the race conditions in > async-timeout. To see how such a race condition can end up with a task never > exiting, take a look at this example: >

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: > Can I also get comments on my proposal for the "scope" parameter? Is there a > use case it would not solve? It looks more complicated -- the extra parameter needs to be passed around via the task and then into the CancelledError exception. What use

[issue46771] Add some form of cancel scopes

Sam Bull added the comment: > 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. > I think

[issue46771] Add some form of cancel scopes

Alex Grönholm added the comment: > Alex, the 'scope' argument can be added if it is really required. > I'm not sure if the nonce is unavoidable still. What other generic solution is there? I've read your last few messages but didn't find an answer. There needs to be some way to avoid a

[issue46771] Add some form of cancel scopes

Andrew Svetlov added the comment: Alex, the 'scope' argument can be added if it is really required. I'm not sure if the nonce is unavoidable still. -- ___ Python tracker ___

[issue46771] Add some form of cancel scopes

Andrew Svetlov added the comment: Updated https://github.com/python/cpython/pull/31394 demonstrated the approach with global dict for counters. -- ___ Python tracker ___

[issue46771] Add some form of cancel scopes

Andrew Svetlov added the comment: In docs we can explain the behavior as "the outer expired timeout cancels the inner waiter, waits for CancelError bubbling up, and raising TimeoutError instead". I agree that a counter is required for this behavior. An alternative implementation can use the

[issue46771] Add some form of cancel scopes

Alex Grönholm added the comment: Can I also get comments on my proposal for the "scope" parameter? Is there a use case it would not solve? -- ___ Python tracker ___

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: The 3rd party context managers are somewhat of a red herring. These are just try/except or try/finally blocks. The inner cm (cm4) is irrelevant, it will see CancelledError and presumably that bubbles out. If it does any resource cleanup we can replace it

[issue46771] Add some form of cancel scopes

Andrew Svetlov added the comment: Suppose we have a case when two nested timeouts are reached at the same event loop iteration: async def asyncio.timeout(1) as cm1: async with third_party_cm() as cm2: async def asyncio.timeout(1) as cm3: async with third_party_cm() as

[issue46771] Add some form of cancel scopes

Change by Tin Tvrtković : -- pull_requests: +29561 pull_request: https://github.com/python/cpython/pull/31415 ___ Python tracker ___

[issue46771] Add some form of cancel scopes

Yury Selivanov added the comment: > * `async with` vs `with`: if Andrew thinks `async with` is easier to teach > and less error-prone, It has to be 'async with' like most asyncio apis to avoid user confusion. -- ___ Python tracker

[issue46771] Add some form of cancel scopes

Andrew Svetlov added the comment: > the third case the behavior is wrong, the `.cancel()` should win over the > timeout. Otherwise using the context manager becomes too risky in real-world > situations. Please elaborate. The first code that calls `.cancel()` wins, doesn't matter what is

[issue46771] Add some form of cancel scopes

Alex Grönholm added the comment: I propose the following, backwards compatible solution: Add a new keyword argument to Task.cancel(): "scope: object = None". The behavior would be as follows: the scope is saved, and included in the raised CancelledError. If Task.cancel() is called again, but

[issue46771] Add some form of cancel scopes

Tin Tvrtković added the comment: Sure, I'll be glad to work with Andrew on getting something presentable. Going through the discussion in the issue: * it seems like folks don't think move_on is useful enough to be in the stdlib, I understand that point. Users can always catch the timeout

[issue46771] Add some form of cancel scopes

Andrew Svetlov added the comment: Guido, the third case: The third edge case is: explicit cancel() happened *after* the timeout (but still in the same iteration)? timeout current-iteration (calls .cancel() after timeout) next-iteration ||

[issue46771] Add some form of cancel scopes

Alex Grönholm added the comment: I am also uncomfortable using the cancel message to deliver the token/nonce/whatever. If TaskGroup.cancel() is implemented, would it also deliver a cancellation in the parent task like trio and AnyIO do? It should IMHO, because otherwise if the task group is

[issue46771] Add some form of cancel scopes

Yury Selivanov added the comment: Couple thoughts: I'm +1 for adding TaskGroup.cancel() method. I'd be -1 on abusing `Task.cancel()` to signal something with a nonce. Whatever problem we are trying to solve here, it should be solvable without resorting to hacks like this. It should be

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: +1 on both aspects of the plan. -- ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue46771] Add some form of cancel scopes

Change by Guido van Rossum : -- pull_requests: +29542 pull_request: https://github.com/python/cpython/pull/31398 ___ Python tracker ___

[issue46771] Add some form of cancel scopes

Andrew Svetlov added the comment: The PR is pretty empty, it has a scaffolding for `asyncio.timeouts` module and its test only. I'll try to add something real tomorrow. My plan is: - solve 'easy' questions with Tin during PR's discussion/reviews - make something that we are both agree on if

[issue46771] Add some form of cancel scopes

Andrew Svetlov added the comment: https://github.com/python/cpython/pull/31394 is created for collaboration , Tin Tvrtković is invited. Core devs should have the write access already. Non-core devs, please ask for github invite if you want to collaborate. --

[issue46771] Add some form of cancel scopes

Change by Andrew Svetlov : -- pull_requests: +29539 pull_request: https://github.com/python/cpython/pull/31394 ___ Python tracker ___

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: Lots of food for thought! There seem to be mostly two discussions: API design for the new asyncio cancel scopes (do we make it more like Trio or more like async-timeout?); and cancel semantics in edge cases. I'll pass on the API design for now: I

[issue46771] Add some form of cancel scopes

Alex Grönholm added the comment: @Guido you asked for the AnyIO implementation of Happy Eyeballs; here it is: https://github.com/agronholm/anyio/blob/ac3e7c619913bd0ddf9c36b6e633b278d07405b7/src/anyio/_core/_sockets.py#L85 (I didn't paste the actual code here because it's way too long)

[issue46771] Add some form of cancel scopes

Tin Tvrtković added the comment: Hello Andrew, here's some followup. About the names: the move_on_after and fail_after names are from Trio. I don't have strong feeling about them at all, whatever most people like. About move_on_after vs loop.call_at and loop.call_later: move_on_after is much

[issue46771] Add some form of cancel scopes

Andrew Svetlov added the comment: I support Alex Grönholm: TaskGroup is not affected by cancellation races because it doesn't convert the exception or swallows it. The code is safe from my understanding. -- ___ Python tracker

[issue46771] Add some form of cancel scopes

Andrew Svetlov added the comment: The discussion is hot, I see several interleaved threads. Let me put my answers on all of them in order of appearance. 1. quattro cancellation scopes are implemented after async-timeout. As the author of async-timeout I am happy to know it. The module is

[issue46771] Add some form of cancel scopes

Tin Tvrtković added the comment: @Guido @Chris Instead of using the message for the nonce we could have a dedicated field for it. I have a proposal though. It's about putting a little logic into the cancellation nonce handling. Let's define the nonce as a float. If you just call

[issue46771] Add some form of cancel scopes

Chris Jerdonek added the comment: > I'm not sure yet (if anything I'd need it for a task, not a future). (By future, I also meant task, as task inherits from future.) For now, I think it would be safer to get the message from the CancelledError, if possible, since how it gets there truly is

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: > > I note that there is no documented way to retrieve the cancel message > Does retrieving it from the CancelledError that is bubbling up suffice? Or do > you need to be able to obtain it from the future object? I'm not sure yet (if anything I'd need it

[issue46771] Add some form of cancel scopes

Chris Jerdonek added the comment: > I note that there is no documented way to retrieve the cancel message Does retrieving it from the CancelledError that is bubbling up suffice? Or do you need to be able to obtain it from the future object? -- nosy: +chris.jerdonek

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: > (I note that there is no documented way to retrieve the cancel message; > you're supposed to access the protected `_cancel_message` attribute, > apparently. Looks like we forgot something there.) Oh, it's passed to the CancelledError() constructor. But

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: FWIW it looks like this part of taskgroups is vulnerable to a similar race: https://github.com/python/cpython/blob/6f1efd19a70839d480e4b1fcd9fecd3a8725824b/Lib/asyncio/taskgroups.py#L212-L232 Deleting the two lines I mentioned won't fix it here; a hack

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: Hm, I see. So the problem is that in the interval between move_on's calls to t.cancel() and t.uncancel(), if the web server calls t.cancel() that will just return False. So the web server would have to implement some other mechanism for cancelling

[issue46771] Add some form of cancel scopes

Tin Tvrtković added the comment: @Guido Imagine something_else() takes a long time and a lot of server resources (like a heavy query). If the web server disconnected a tad later and avoided the race condition, the task would have gotten cancelled very soon after the start of something_else()

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: @Tin The sad path is just a race, right? If the web server had disconnected just a tad later, __aexit__() would already have returned and await something_else() would already be running. So you can't make any promises if you write your code that way

[issue46771] Add some form of cancel scopes

Alex Grönholm added the comment: I just tried to write a snippet to demonstrate the issue in TaskGroup, but that doesn't seem possible since TaskGroup never swallows a CancelledError. It always raises/propagates _some_ exception out of __aexit__() unless of course all the child tasks run to

[issue46771] Add some form of cancel scopes

Tin Tvrtković added the comment: Ok, first the happy path. We have a task running this: async def a(): with move_on_after(5): await a_long_op() await something_else() `move_on_after` has scheduled a callback to run that calls `parent_task.cancel(msg=1)` 5 seconds after it was

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: I hesitate to add yet another stack at this fundamental level. The Task.cancel() method returns a bool which indicates whether any state changed. When multiple cancellations happen concurrently, all but the first will return False, and anything that

[issue46771] Add some form of cancel scopes

Tin Tvrtković added the comment: On the topic of TaskGroups needing to know whether to swallow a child CancelledError or no: just use the same nonce/cancellation context trick? I remember asking Nathaniel about why in Trio nurseries and cancel scopes were linked (nurseries contain a cancel

[issue46771] Add some form of cancel scopes

Tin Tvrtković added the comment: The use of the cancel message is a hack, yeah. But it's what I had to work with. We could introduce a new, proper cancellation context to Tasks instead, which could be attached to the CancelError when it's raised. On the topic of multiple cancellations being

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: Oh, wait. The new Task.cancelling() API helps tell the difference between the *parent* task being cancelled "from outside" vs. the task group itself cancelling the parent task so as to break out of an await like the following: async with TaskGroup() as g:

[issue46771] Add some form of cancel scopes

Change by Irit Katriel : -- nosy: +ajoino ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue46771] Add some form of cancel scopes

Alex Grönholm added the comment: 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

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: OK. 1. 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. 2. Suppose you could make (backwards compatible) changes to asyncio. What would

[issue46771] Add some form of cancel scopes

Alex Grönholm added the comment: 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

[issue46771] Add some form of cancel scopes

Change by Jacob Nilsson : -- nosy: +ajoino ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue46771] Add some form of cancel scopes

Change by Joshua Bronson : -- nosy: +jab ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: 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

[issue46771] Add some form of cancel scopes

Change by Guido van Rossum : -- nosy: +tinchester ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue46771] Add some form of cancel scopes

Alex Grönholm added the comment: 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

[issue46771] Add some form of cancel scopes

Guido van Rossum added the comment: 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

[issue46771] Add some form of cancel scopes

Tin Tvrtković added the comment: Oh, and Trio's `current_effective_deadline` is also in. -- ___ Python tracker ___ ___

[issue46771] Add some form of cancel scopes

Tin Tvrtković added the comment: 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: * the API follows Trio, so instead of `timeout` you'd use `fail_after` or

[issue46771] Add some form of cancel scopes

Change by Guido van Rossum : -- keywords: +patch pull_requests: +29526 stage: needs patch -> patch review pull_request: https://github.com/python/cpython/pull/31270 ___ Python tracker

[issue46771] Add some form of cancel scopes

New submission from Guido van Rossum : 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