Discussion: Cancel-myself-now operation? cancel-and-wait-for-it-to-finish operation?

[belm0]

it isn't apparent from the docs that cancel() is non-raising, and that when you call it from within a nursery the program flow will continue until the next checkpoint

it seems like a common use case will be to abort from within a nursery scope using cancel() and sleep(0). Perhaps nursery should have a cancel_now() or abort() method

my use case was related to having a task meet the start() protocol invariant of either calling started or raising

[belm0]

this documentation adds to the confusion:

Attempting to raise Cancelled yourself will cause a RuntimeError. It would not be associated with a cancel scope and thus not be caught by Trio. Use cancel_scope.cancel() instead.

which suggests that cancel_scope.cancel() would be a suitable solution for the case when you want to immediately stop execution from within a nursery scope.

[belm0]

is there a way to make raise Cancelled from within a nursery scope just work? 🙏

[njsmith]

We should definitely be clearer about cancel not being a checkpoint in the docs – it's logical and internally consistent but surprising to folks coming from other systems. It's actually a good example for driving home the right way to think about trio cancellation.

If we do add a method that does cancel+checkpoint, then the cancel scope would be a more obvious place for it than the nursery, because it's generic cancellation-related functionality.

my use case was related to having a task meet the start() protocol invariant of either calling started or raising

I'm having some trouble visualizing this... It's one of those times where the use case is super clear in your head because you're looking at it, but the rest of us have a bit more trouble :-). I guess you're... starting a task, and also passing in the nursery where start was called, and sometimes during startup the task decides to abort everything in that nursery, and this isn't an error so it can't raise an exception, but it's also not a success so it can't call started? Is it... something like that?

[njsmith]

is there a way to make raise Cancelled from within a nursery scope just work? 🙏

Nurseries can be nested, so how would trio know which nursery you were throwing the Cancelled to?

[belm0]

Nurseries can be nested, so how would trio know which nursery you were throwing the Cancelled to?

Assuming we added another cancel-like exception for this case, it would apply to the first nursery catching it. That manager would convert it to a cancel scope + checkpoint.

I'm having some trouble visualizing this...

The specific use case was was trying to upgrade this task in trio-websocket to used the start() protocol, and have it call started() after the HTTP handshake is done. Instead of the break statements in the code I wanted to abort the nursery scope. (Granted this may all go away with trio-websocket design improvements.)

https://github.com/HyperionGray/trio-websocket/blob/94fc77b863f70b4d15c16bf44b602224c5b5426f/trio_websocket/__init__.py#L255-L285

[njsmith]

trio-websocket

Agreed that it's probably better to do the handshake before starting that task. But also: I don't see why you can't just call started there? Or raise an exception? (Note that if you raise an exception before calling started, it comes out of the call to start, not the normal nursery path, so your startup code can catch it and handle it.)

[njsmith]

it would apply to the first nursery catching it.

We really try to avoid constructs like this, where the result depends on some subtle aspect of the context where the code is called. It creates non-obvious coupling and breaks compositionally/separation of concerns.

And this case is actually a really great example of that :-). As it turns out, start internally creates a temporary nursery, spawns the task into that temporary nursery, and then later moves it to the final nursery (when you call started). This gives nice semantics for start being cancelled or an early exception.

We can get away with this, because there's no way for user code to tell that there's a temporary nursery in there (modulo debugging APIs). With your raise Cancelled idea though, this internal implementation trick would become exposed as part of the public API. Also, it means that in your specific case, the nearest nursery actually isn't the one you expected!

[belm0]

Thank you for the explanation. I think the priorities are:

1. clarify cancel() behavior in the docs
2. improve documentation note about explicitly raising Cancelled(), suggesting sleep(0) after cancel() when requesting the cancel from within the cancel scope to have it end program flow
3. possibly add a cancel + sleep convenience method to cancel scope

[smurfix]

Well, we could have cancel() return something with an __await__ attribute, so that await scope.cancel() would actually be that "cancel+sleep" convenience method.

Probably not a good idea from the PoV of verifying code correctness, but …

[oremanj]

Confusing the matter further, the "cancel and immediately raise" operation only makes sense if it's called from within the cancel scope. In general, one task can cancel some work in another task just fine, but it wouldn't make sense to perform a naive implementation of this "cancel immediately" operation across tasks.

If we go with await cancel_scope.cancel(), we could add the semantics that if invoked from a different task, the await sleeps until the cancel scope actually gets exited. That seems fairly ergonomic and like it might even be useful sometimes, but maybe a bit of a pain to implement compared to other approaches.

In general trio tends to eschew foo() and await foo() both being valid statements that do different things, so we might want a different name: wait_cancelled()? cancel_and_wait()?

The other possible API would be something that does cancel() and raises a Cancelled exception for that scope. It would not need to be async, but would only make sense inside the cancelled scope, so there would need to be a check for that. cancel_and_raise()?

I'd prefer not to reuse the term "abort" as that already has a meaning in trio (waking up a sleeping task to deliver a Cancelled or a KeyboardInterrupt exception).

[belm0]

Having a separate async cancel method that does the right thing both inside and outside of the cancel scope sounds great.

The naming will have to be a compromise, cancel_and_wait() sounds like the best we could do. Ideally we'd follow trio naming conventions (cancel() and cancel_nowait()), but that would be an API breakage nightmare. While cancel_and_wait doesn't make it obvious that it will always raise when inside the cancelled scope, I think we'll get used to it.

[njsmith]

cancel_and_wait

Interesting idea.

This would interact awkwardly with #607, since an unbound cancel scope might never be entered, and thus never exited either. My intuition is that if we have to pick, then unbound cancel scopes are more useful than the ability to wait for a cancel scope to exit, mostly because I've noticed cases where I wanted unbound cancel scopes but not noticed cases where I wanted to wait for a cancellation. But maybe that's because I was looking :-).

I still don't have any intuition for why cancel_and_checkpoint is an important shorthand to support. Using cancel_and_wait to handle both waiting for the cancel scope and also raising immediately is clever, but it seems confusing to have a single method designed to perform opposite operations like this...

[smurfix]

cancel_and_wait

Meh. If you're inside the scope to be cancelled, this doesn't wait – it raises Cancelled.

I don't want a single method that affects control flow in two very different ways (and is named for only one of them); if we do that, we might as well have a cancel method that can be awaited (or not).

If we don't want that (and I'm no longer sure whether keeping three names in mind is any worse than keeping three ways of using .cancel in the back of your brain, at least not in this case) we might add something like a cancel_me method that raises a RuntimeError if it's acting on a cancellation scope which the current task is not a member of. This method will thus never succeed, thus it behaves and is named differently than cancel_and_wait.

This would interact awkwardly with #607, since an unbound cancel scope might never be entered, and thus never exited either.

Well, I'd wait only for the tasks to exit that are currently within the scope; if none are, return immediately.

[oremanj]

If you're inside the scope to be cancelled, this doesn't wait – it raises Cancelled.

Just like anything else that blocks inside a cancelled cancel scope. :-) Though I understand the potential for confusion.

I'd wait only for the tasks to exit that are currently within the scope; if none are, return immediately.

AIUI, unbound cancel scopes are intended to fix races where you might be cancelling before the scope gets entered, so having cancel_and_wait() return before the scope gets entered could be confusing. It seems hard to pick one behavior (wait for the scope to be exited VS return immediately if not entered yet) that would be intuitive in all cases.

if we have to pick, then unbound cancel scopes are more useful than the ability to wait for a cancel scope to exit

I agree - I don't have specific use cases for "wait for cancel scope to exit" either, and it would entail some internal complexity / arguably some dependency inversion (cancel scopes currently don't have anything async going on).

I'd support cancel_me(), or maybe call it cancel_noreturn() or cancel_and_raise() or something else, implemented as a synchronous function. That provides a useful behavior (synchronously raising a Cancelled exception) that's not possible to obtain with other public APIs, and would have a straightforward implementation.

[belm0]

I'd support cancel_me(), or maybe call it cancel_noreturn() or cancel_and_raise() or something else, implemented as a synchronous function.

I think it's fair to revisit async options if some real use cases show up.

+1 for "cancel and raise" method, though it really needs the RuntimeError safeguard mentioned by @smurfix for "I'm not within the scope being cancelled".