-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Proposal: Task cancellation shields #3037
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
a611405
2f5a442
575b0fd
6a20923
7537b61
1357c93
24a4e80
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,340 @@ | ||||||||||||||||||||||||||||||||||||
| # Task Cancellation Shields | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| * Proposal: [SE-NNNN](NNNN-task-cancellation-shields.md) | ||||||||||||||||||||||||||||||||||||
| * Authors: [Konrad 'ktoso' Malawski](https://github.com/ktoso) | ||||||||||||||||||||||||||||||||||||
| * Review Manager: TODO | ||||||||||||||||||||||||||||||||||||
| * Status: TODO | ||||||||||||||||||||||||||||||||||||
| * Implementation: [PR #85637](https://github.com/swiftlang/swift/pull/85637) | ||||||||||||||||||||||||||||||||||||
| * Review: | ||||||||||||||||||||||||||||||||||||
| * TODO | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| ## Introduction | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| This proposal introduces a new mechanism to temporarily "ignore" task cancellation, called task cancellation shields. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| This can be used to ensure certain pieces of code will execute regardless of the task's cancelled status. A common situation where this is useful is running clean-up code, which must execute regardless of a task's cancellation status. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| This proposal dovetails nicely with asynchronous defer statements which were recently introduced in [SE-0493: Support `async` calls in `defer` bodies](https://github.com/swiftlang/swift-evolution/blob/main/proposals/0493-defer-async.md), which are frequently used to express such resource clean-up functionality. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| ## Motivation | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Task cancellation is _final_ and can not be ignored or undone. Once a task has been cancelled, it remains cancelled for the rest of its existence. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Child tasks are also affected by task cancellation, and cancellation propagates throughout the entire task tree, allowing for efficient and holistic cancelling of entire hierarchies of work, represented as a tree of child tasks. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Today, there is no great way to ignore cancellation, and some pieces of code may therefore by accident not execute to completion. This is especially problematic in clean-up or resource tear-down, where a tear-down method's implementation details might be checking for cancellation, however, we _must_ have this code execute, regardless of the task's cancellation status to properly cleanup some resource, like this: | ||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
maybe refrain from using 'ignore' here since above it was just stated above that cancellation cannot be ignored. unless that preface was intentional to contrast the existing state with how this feature lets you 'ignore' cancellation in some sense. though still i'm not sure that's the characterization we want to be encouraging. maybe something along these lines:
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| ```swift | ||||||||||||||||||||||||||||||||||||
| extension Resource { | ||||||||||||||||||||||||||||||||||||
| func cleanup() { // our "cleanup" implementation looks correct... | ||||||||||||||||||||||||||||||||||||
| system.performAction(CleanupAction()) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| 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 | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| // ... | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| In the above example, while we may have implemented the resource clean-up correctly, we may be unaware of the system only performing actions while the task it is executing in is not cancelled. In order to ensure certain actions execute regardless if called from a cancelled or not cancelled task, we're going to have to "shield" the cleanup code from observing the cancellation status of the calling task. | ||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Today, developers work around this problem by creating unstructured tasks, which creates unnecessary scheduling and may have a performance and even correctness impact on such cleanup code: | ||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: since the issues are covered in the list below, maybe just introduce the workaround first rather than repeating them:
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| ```swift | ||||||||||||||||||||||||||||||||||||
| // WORKAROUND, before cancellation shields were introduced | ||||||||||||||||||||||||||||||||||||
| func example() async { | ||||||||||||||||||||||||||||||||||||
| let resource = makeResource() | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| assert(Task.isCancelled()) | ||||||||||||||||||||||||||||||||||||
| await Task { | ||||||||||||||||||||||||||||||||||||
| assert(!Task.isCancelled()) | ||||||||||||||||||||||||||||||||||||
| await resource.cleanup() | ||||||||||||||||||||||||||||||||||||
| }.value // break out of task tree, in order to ignore cancellation | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+53
to
+61
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: code didn't compile & the first assertion seems a bit confusing. also, removed the use of 'ignore'
Suggested change
|
||||||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| This is sub-optimal for a few reasons: | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| - We are introducing an unstructured task which needs to be scheduled to execute, and therefore delaying the timing when a cleanup may be executed, | ||||||||||||||||||||||||||||||||||||
ktoso marked this conversation as resolved.
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||
| - It is not possible to use this pattern in a synchronous function, as we need to await the unstructured task. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Task cancellation shields directly resolve these problems. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| ## Proposed solution | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| We propose the introduction of a `withTaskCancellationShield` method which temporarily prevents code from **observing** the cancellation status, and thus allowing code to execute as-if the surrounding task was not cancelled: | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| ```swift | ||||||||||||||||||||||||||||||||||||
| public func withTaskCancellationShield<Value, Failure>( | ||||||||||||||||||||||||||||||||||||
| _ operation: () throws(Failure) -> Value, | ||||||||||||||||||||||||||||||||||||
| file: String = #fileID, line: Int = #line | ||||||||||||||||||||||||||||||||||||
| ) throws(Failure) -> Value | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| public nonisolated(nonsending) func withTaskCancellationShield<Value, E>( | ||||||||||||||||||||||||||||||||||||
| _ operation: nonisolated(nonsending) () async throws(Failure) -> Value, | ||||||||||||||||||||||||||||||||||||
| file: String = #fileID, line: Int = #line | ||||||||||||||||||||||||||||||||||||
| ) async throws(Failure) -> T | ||||||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Shields also prevent the automatic propagation of cancellation into child tasks, including `async let` and task groups. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| They do not prevent a task from being cancelled, however, they affect the observation of the cancelled status while executing in a "shielded" piece of code. This is best explained with an example: | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| ```swift | ||||||||||||||||||||||||||||||||||||
| assert(Task.isCancelled) // 🛑 | ||||||||||||||||||||||||||||||||||||
| withTaskCancellationShield { | ||||||||||||||||||||||||||||||||||||
| assert(Task.isCancelled == false) // 🟢 | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| assert(Task.isCancelled) // 🛑 | ||||||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+91
to
+97
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i find this example kind of confusing.
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| ### Cancellation Shields and Child Tasks | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Cancellation shielding also prevents the automatic propagation of the cancellation through the task tree. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Specifically, if a structured child task is created within a task cancellation shield block and the outer task is canceled, the outer task will be canceled. However, we will not observe this flag change until we exit the cancellation shield. At the same time, the child tasks which are running within the task cancellation shield will not become canceled automatically, as would be otherwise the case: | ||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: 'canceled' vs 'cancelled' consistency
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| ```swift | ||||||||||||||||||||||||||||||||||||
| Task { | ||||||||||||||||||||||||||||||||||||
| withUnsafeCurrentTask { $0?.cancel() } // immediately cancel the Task | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // without shields: | ||||||||||||||||||||||||||||||||||||
| async let a = compute() // 🛑 async let child task is immediately cancelled | ||||||||||||||||||||||||||||||||||||
| await withDiscardingTaskGroup { group in // 🛑 task group is immediately cancelled | ||||||||||||||||||||||||||||||||||||
| group.addTask { compute() } // 🛑 child task is immediately cancelled | ||||||||||||||||||||||||||||||||||||
| group.addTaskUnlessCancelled { compute() } // 🛑 child task is not started at all | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // with shields: | ||||||||||||||||||||||||||||||||||||
| await withTaskCancellationShield { | ||||||||||||||||||||||||||||||||||||
| async let a = compute() // 🟢 async let child task is NOT cancelled immediately | ||||||||||||||||||||||||||||||||||||
| await withDiscardingTaskGroup { group in // 🟢 not cancelled | ||||||||||||||||||||||||||||||||||||
| group.addTask { compute() } // 🟢 not cancelled | ||||||||||||||||||||||||||||||||||||
| group.addTaskUnlessCancelled { compute() } // 🟢 not cancelled | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| However if a child task (or entire task group) were to be cancelled explicitly, the shield of the parent task has no effect, as it only shields from "incoming" cancellation from the outer scope and not the child task's own status. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| ```swift | ||||||||||||||||||||||||||||||||||||
| await withTaskCancellationShield { | ||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||
| await withDiscardingTaskGroup { group in | ||||||||||||||||||||||||||||||||||||
| group.addTask { ... } | ||||||||||||||||||||||||||||||||||||
| group.cancelAll() // cancels all tasks within the group, as expected | ||||||||||||||||||||||||||||||||||||
ktoso marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| It is meaningless to try to shield the `addTask` operation of a task group as it does not enclose the lifetime or any part of the child tasks execution. Instead you should shield the child task within the `addTask` function if shielding a specific task is your goal: | ||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| ```swift | ||||||||||||||||||||||||||||||||||||
| await withDiscardingTaskGroup { group in | ||||||||||||||||||||||||||||||||||||
| // ❌ has no effect on child task observing cancellation: | ||||||||||||||||||||||||||||||||||||
| withTaskCancellationShield { | ||||||||||||||||||||||||||||||||||||
| group.addTask { ... } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // 🟢 does properly shield specific child task observing cancellation: | ||||||||||||||||||||||||||||||||||||
| group.addTask { | ||||||||||||||||||||||||||||||||||||
| withTaskCancellationShield { ... } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| All examples shown using `isCancelled` behave exactly the same for `Task.checkCancellation`, i.e. whenever `isCancelled` would be true, the `checkCancelled` API would throw a `CancellationError`. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| ### Cancellation Shields and Cancellation Handlers | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Swift concurrency offers task cancellation handlers which are invoked immediately when a task is cancelled. This allows you to dynamically react to cancellation happening without explicitly checking the `isCancelled` property of a task. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Task cancellation shields also prevent cancellation handlers from firing if the handler was stored while a shield was active. Again, this does not extend to child tasks, but only to the current task that is being shielded. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| For example, the task cancellation shield installed around the `slowOperation` in the snippet below, would effectively prevent the cancellation handler inside the `slowOperation` function from ever triggering: | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| ```swift | ||||||||||||||||||||||||||||||||||||
| func slowOperation() -> ComputationResult { | ||||||||||||||||||||||||||||||||||||
| await withTaskCancellationHandler { | ||||||||||||||||||||||||||||||||||||
| return < ... slow operation ... > | ||||||||||||||||||||||||||||||||||||
| } onCancel: { | ||||||||||||||||||||||||||||||||||||
| print("Let's cancel the slow operation!") | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| func cleanup() { | ||||||||||||||||||||||||||||||||||||
| withTaskCancellationShield { | ||||||||||||||||||||||||||||||||||||
| slowOperation() | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| ### Cancellation Shields and Task handles | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Unstructured tasks, as well as the use of `withUnsafeCurrentTask`, offer a way to obtain a task handle which may be interacted with outside of the task. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| For example, you may obtain a task handle a an unstructured task, which then immediately enters a task cancellation shield scope: | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
| For example, you may obtain a task handle a an unstructured task, which then immediately enters a task cancellation shield scope: | |
| For example, you may obtain a task handle to an unstructured task, which then immediately enters a task cancellation shield scope: |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| The **instance method** `task.isCancelled` queried from the outside of the task will return the _actual_ cancelled state, regardless if the task is right no executing a section of code under a cancellation shield or not. This is because from the outside it would be racy to query the cancellation state and rely on wether or not the task is currently executing a section of code under a shield. This could lead to confusing behavior where querying the same `task.isCancelled` could be flip flopping between cancelled and not cancelled. | |
| The **instance method** `task.isCancelled` queried from the outside of the task will return the _actual_ cancelled state, regardless of whether the task is currently executing a section of code under a cancellation shield or not. This is because from the outside it would be racy to query the cancellation state and rely on whether or not the task is currently executing a section of code under a shield. This could lead to confusing behavior where querying the same `task.isCancelled` could be flip flopping between cancelled and not cancelled. |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
queried from the outside of the task
this suggests to me the behavior would be different if queried from within the task itself – is that assumption correct? if not, maybe best to drop the condition.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Therefore the static
Task.isCancelledmethod is always returning the actual cancellation status (regardless of installed shields).
i don't follow what this is trying to communicate. my understanding was that the static method would not return the actual cancel state, but something like the 'current context's semantically-appropriate cancellation state'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| In order to aid understanding and debuggability of cancellation in such systems, we also introduce a new property to query for a cancellation shield being active in a specific task. | |
| In order to aid understanding and debuggability of cancellation in such systems, we also introduce a new property to query for an active cancellation shield in a specific task. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| This API is not intended to be used in "normal" code, and should only be used during debugging issues with cancellation, to check if a shield is active in a given task. This API are _only_ available on `UnsafeCurrentTask`, in order to dissuade from their use in normal code. | |
| This API is not intended to be used in "normal" code, and should only be used during debugging issues with cancellation, to check if a shield is active in a given task. This API is _only_ available on `UnsafeCurrentTask`, in order to dissuade from its use in normal code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is an instance method but the above suggests there will only be a static method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit/question: are 'status' and 'state' synonymous in this context? personally i find 'state' more clear, though i see they each have roughly equal use ('status' vs 'state') within this repo at the moment (and i know 'status' is a term within the runtime implementation).