[Pitch] Task Cancellation Shields

Evolution Pitches
23

I’m not convinced by the “long name” for this specific API because the name, even though long and descriptive, doesn’t capture the behavior and typical use-case: making sure that even inside a cancelled task, we’re not going to observe the cancellation:

// cancelled = true
withTaskCancellationShield { 
  // cancelled = false
}

This is more than only disabling the propagation, so we’d have to capture that in a descriptive name if we wanted one, and that becomes a bit unwieldy.

I’d actually favor the term shielding here and just offer good documentation both in the proposal and API. At least on APIs we’re able to give good easy to access docs, which doesn’t translate very well to language features without APIs, but here we have a good place to document it in depth.

The shield name does come from prior art in trio (noted in the proposal), which sometimes has some colorful API names we did choose to not adopt (“nurseries” which are our task groups), however in this case I think the name is pretty good. If someone came up with a really compelling name though we could consider it (including names for the is…Active APIs).

2 Likes
24

This isn’t what I meant, clearly this is “implicit” because the user didn’t write it in source, so yes it is implicit. What I meant that it is not caused by cancellation propagation and therefore is a different path the cancellation happens.

25

It never meant that; It specifically always meant “this specific task is cancelled”, and the cancellation propagation caused individual tasks in a hierarchy to become cancelled. We’ve been consistently explaining this since early proposals and also in WWDC sessions.

26

I have a small question why not use this for cleaning up resources when a task is canceled, because what I understood from the proposel is that you have a case where the parent task got cancelled and due to that sub tasks couldn’t clean up resources

func withTaskCancellationHandler<T>(
    operation: () async throws -> T,
    onCancel handler: () -> Void,
    isolation: isolated (any Actor)? = #isolation
) async rethrows -> T

Example used in the proposal:

extension SomeSystem { 
  func performAction(_ action: some SomeAction) { 
    guard Task.isCancelled else {
      // oh no! 
      // If Resource.cleanup calls this while being in a cancelled task,
      // the action would never be performed!
      return 
    }
    // ... 
  }
}
27

That doesn’t help in any way because this is about code you don’t control that may need to execute, but that code is doing the is cancelled check.

If you’re suggesting to put the cleanup logic into both normal path and the onCancel, this makes things even more complicated and racy – you may get cancelled while in the operation and need to ensure the cleanup runs only once etc.

28

this makes things even more complicated and racy

Why is that ?

29

I have to guess what exact pattern you have in mind since you didn’t write it up, but I assume you’re suggesting some form of:

withTaskCancellationHandler {
  bla()
  cleanup()
} onCancel: {
  cleanup()
}

This absolutely could end up running the cleanup twice. So you’d have to enforce single execution using some synchronization mechanism – in other words, much more complicated.

If you have some other specific ideas, please write them up so we can discuss a specific code pattern.

1 Like
30

withTaskCancellationHandler {
bla()
cleanup()
} onCancel: {
cleanup()
}

This is exactly the pattern I have in mind, you can easily make it safe by making this method `cleanup()` isolated you are right it may be complex

31

But if we wrote it in this way

@HeavyWorkActor
class HeavyWork {
    func heavyWork() async {
        await withTaskCancellationHandler {
        bla()
        } onCancel: {
        cleanup()
        }
    }

     func doTheWork() async {
        await heavyWork()
        // other work
        cleanup()
    }

    private func cleanup() { // this will be isolated ehance protected
        
    }
}

There will be no problem I think

32

As I understand it, the onCancel handler in withTaskCancellationHandler is:

  • synchronous
  • without any guaranteed isolation
  • executed by the task/thread cancelling the task
  • possibly while holding task related locks

so doing any complex cleanup inline there is not advisable and impossible if the cleanup is async.

1 Like
33
    1. synchronous
    2. without any guaranteed isolation
    3. executed by the task/thread cancelling the task
    4. possibly while holding task related locks

    this code above is compiling with swift 6 which means that cleanup() is isolated and safe. this what I understand please correct me if I am wrong

impossible if the cleanup is async.

you are right about this but do we truly need a clean up method to be async ?

34

The purpose of this feature is specifically to support cleanups and async code.

The snippet above cannot work, the onCancel callback is synchronous and NOT isolated to the same context – in order to call the isolated cleanup you’d have to suspend, so the snippet you shared won’t compile anyway, regardless if the cleanup is sync or async.

Relying on the onCancel calling task to not be cancelled also doesn’t really work, it may also be cancelled, so again – you need a shield there.

I think we can put this idea to rest, but thank you for bringing up alternate ideas.

1 Like
35

thanks man for this explanation and also thanks for correcting me :heart:

1 Like
36

i see. well, now that i think i understand the model a bit better, here are a few more thoughts/questions:

i think overall the proposal could make it more clear that a 'shield' applies only to a single Task despite the lexical structure sort of suggesting that it will fully 'cascade' through to children (assuming that's correct...). i think this is communicated in the current text, but, at least personally, i did not find that self-evident. perhaps drawing a distinction with the manner in which task local scoping works would help?

this seems rather niche, but what is the intended behavior if a cancellation handler formed 'outside' a shielded region is executed from a 'self cancel' occurring within such a region?

await withTaskCancellationHandler {
  await withTaskCancellationShield {
    withUnsafeCurrentTask {
      print("(shield) cancelled: ", Task.isCancelled) // false (not cancelled)
      $0?.cancel() // cancel handler synchronously invoked (i think)
      print("(shield) cancelled: ", Task.isCancelled) // false (shield)
    }
  }
} onCancel: {
  print("cancelled: ", Task.isCancelled) // ?? false b/c it runs in the shield?
}
1 Like
37

There's no special behavior: the task is still cancelled, which synchronously triggers any unshielded cancellation handlers (handlers entered outside of any shield).

1 Like
38

to confirm then – so the query of the cancellation state from within the handler should return false in the example?

39

Yes, that's right. I can see how someone might think of it as a surprising special case, but I think it's okay. Task.isCancelled is an odd thing to call within a cancellation handler anyway, because that's reading the state of the task that's doing the cancellation, which is not generally something you're supposed to be reasoning about.

2 Likes
40

+1 on the proposal in general, I've run into this problem in the past and while I was able to work around it (since I also owned the code doing the cleanup), the solution ended up being quite hacky. At the time I hoped for a more robust way of preventing cleanup work from being cancelled, and this proposal achieves just that.

I had a similar thought when I first read the proposal text. It's clear what shields do if you've read through the proposal, but if I were to find a withCancellationShield { ... } block "in the wild", not having read the proposal, I wouldn't immediately think that it works by changing the value of Task.isCanceled within the block. So I'd like to challenge the need for a term of art here too.

Since the authors seem open to considering alternative names, may I ask what you all think about withTaskCancellationDisabled { ... }? With the alternative for the proposed isTaskCancellationShielded API naturally being isTaskCancellationDisabled?

Another quibble I have against using "shield" (or any other noun) in the name is that in most Swift APIs of the form with[noun] { ... }, you can get a hold of the thing referenced in the name. And I think that greatly aids understanding: if I'm unsure about what, say, withTaskGroup does, I can just inspect the TaskGroup instance provided in the closure and see its documentation!

That is true for most of the APIs in that form (as far as I can tell[1]), the main exception being APIs of the form with[...]Handler { ... }[2] where the "handler" is just a closure. But then I think it's generally understood that a "handler" can be a closure.

However, in this case, there's no TaskCancellationShield type. In a way, a shield is not really a thing that exists. As a consequence, I can't go look up its documentation and get a description of what cancellation shields are and what they do.

(And it seems to me like it's not even possible to design an API where a "shield" is a meaningful type, because it's essentially just a boolean value in the state of the Task, which I think makes the case that the API really doesn't need a new term of art).

Also, minor note: while reading through the proposal I struggled with the concept of an "active" cancellation shield. Is a shield "active" just because there's one in the current context (ie simply because we're inside a withCancellationShield block), or is a shield "active" when we're inside a withCancellationShield block and the enclosing task is cancelled (ie the shield is "actively" changing the value of Task.isCancelled)? At first, I would have thought it was the latter!

  1. withTaskGroup, withCheckedContinuation, withUnsafePointer, withSpan, withUnsafeCurrentTask, withUnsafeMutableBytes, withContiguousStorageIfAvailable, withUTF8... ↩︎

  2. withTaskCancellationHandler, withTaskPriorityEscalationHandler... ↩︎

4 Likes
41

personally i don't think 'disabled' would be an appropriate term, because that doesn't seem like it's exactly what's happening[1]. it's not that cancellation is disabled – all the Tasks can still be cancelled – it's that the default cancel propagation to children is disabled, and the behavior of the cancel status observation APIs is changed. i think the fact that two different but important things are being done by the same API makes finding a good, concise name kind of challenging.

  1. though i am also sympathetic to 'shield' not being an immediately compelling choice, but also haven't thought of anything better ↩︎

42

I don’t have anything special to say about the semantics, all seems fine.

Regarding the name, it sort of sounds like it’s introducing this “shield” concept to the language. I think something simpler, such as ignoringCancellation {}, would be clearer at first sight. Or if you want to be super clear that tasks inside the closure can still be cancelled, just not by the outer task, it could be spelled clearly with ignoringOuterCancellation {}, without having to learn what “shield” means since it’s all spelled out.