SE-0504: Task Cancellation Shields

In its discussion of SE-0504, the LSG decided that we'd like more feedback about the suggestion above that the proposed hasActiveTaskCancellationShield property might be more appropriate as a static property on Task rather than just as an instance property of UnsafeCurrentTask. When we brought this up with the author, he chose to amend the proposal in line with the suggestion.

The review of SE-0504 is therefore extended until February 2nd to give the community an opportunity to provide feedback on this point. I would like to specifically encourage feedback about whether the API should be offered in both places, or whether it would be better to only offer it in one way or the other.

John McCall
Review Manager

3 Likes

-1 for Task.hasActiveTaskCancellationShield public API.

When seeing from effect system's point of view,
reinterpretation of cancellation should simply stick with reinterpretation only without adding extra states (which in this case is hasActiveTaskCancellationShield),
and reinterpretation should normally be unobservable from the callee.

Also consider if we have Task.hasActiveTaskCancellationShield,
we may also need try Task.checkActiveTaskCancellationShield(),
as well as in the worst case, withHasActiveTaskCancellationShieldShiled (shielding the shield),
withHasActiveTaskCancellationShieldShiledShield, etc, which is an endless rat race.

5 Likes

I am personally not convinced that we should offer a public API on Task to check if a shield is active like the proposed hasActiveTaskCancellationShield. I can already see that somewhere code is ending up checking this not only for debugging purposes but also for changing control flow, which would entirely defeat the purpose of a shield in my opinion. I understand though that it is strange to offer this on UnsafeTask only since nothing about it is unsafe. Have we considered surfacing this information for debugging purposes through the Task.description and Task.debugDescription properties? While this would still allow developers to parse the String, it presents a lot higher barrier than a simple Task.hasActiveTaskCancellationShield.

9 Likes

Ugh I have been bitten by developers parsing debug strings, it effectively makes that debug string ABI. I would suggest against that and instead teach lldb to read the task structure around if it has an active shield or not.

The example behind my objection to that particular case is NSData's description used to have the full bytes in hex, we changed that for some performance reasons and end up breaking a ton of apps and had to revert a seemingly valid and meaningful performance improvement because some high profile users were taking advantage of the description. If we intend for it only to be used for debugging then we should only have it available there or at the bare minimum claim the debug description is documented heavily to NOT use it or parse it at runtime.

I'm familiar with the change you're talking about (it broke push token handling in quite a few iOS apps), and it really speaks to two lessons.

First, in the NSData / Data case, the description was used because there was no built-in way of getting a hex string for easy use in things like backend APIs to track push tokens. So if we don't want people to parse the description to see whether there's a shield, we need to provide some official way of determining the same. If we're okay with people parsing it from description, it seems like we should be okay with an official API.

Second, for the push token case, it really should've returned an opaque value with its own API surface to get a String representation, rather than returning raw Data. Unfortunately the opportunity to apply this lesson and expose opaque debug types for debugger consumption has passed in Swift.

So I guess I'm saying, if this info is visible in more than the debugger, it should be exposed through API we want to be used, rather than that we hope is not.

2 Likes

I’m very against making things only accessible through weird hacks and workarounds on purpose, it honestly makes such designs broken or at best ā€œincompleteā€, which is a bad look that affects how people who want to get work done perceive Swift.

Specifically, I’m very against ideas that ā€œsome information is only accessible in a magic string somewhereā€:

Making lldb print Task information like this, as well as swift-inspect (and other tools), is very valuable and we’ll be pursuing that in any case, but it’s not really part of this discussion. lldb and other tools should always be able to show any and all information about tasks, and we’re continuing to make that work whenever we add things to tasks.

To clarify, these would be arguments against having this API available at all and would not be specific only to namespacing under Task versus UnsafeCurrentTask, right?

Ideally, UnsafeCurrentTask.hasActiveTaskCancellationShield is also NO for me,
but since I still don't know about the happy usage from compiler side,
I'm fine with having it as far as "Unsafe" is explicitly stated
so that public "shield-of-shield" APIs are unlikely requested in future.

Therefore the API documentation will be changed to reflect this change:

/// ... 
/// A task's cancellation is final and cannot be undone.
/// However, it is possible to cause the `isCancelled` property to return `false` even 
/// if the task was previously cancelled by entering a ``withTaskCancellationShield(_:)`` scope.
/// ...
public var isCancelled: Bool {

The instance method task.isCancelled retains its existing behavior.

I don't love that the documentation for Task.isCancelled now needs to take a detour to explain the niche case of task cancellation handlers. But since it does, I hope the updated documentation also mentions that this is true for Task.isCancelled but not for task.isCancelled.

(And since they no longer mean the same thing, perhaps the documentation should explicitly spell out Task.isCancelled instead of just saying "isCancelled" wherever it could cause confusion).


Even if the proposed API is deemed worthy of a new term of art, this debug-only property seems to be adding yet another concept of top of it by introducing the concept of a shield being "active", that isn't described anywhere else. What does it mean for a shield to be "active"? Is it active because it's "actively" protecting a block of code from potential cancelation? Or is it active only when it's actually changing the value of Task.isCancelled from what it'd be if the shield wasn't there?

It seems like at some point it was just called hasTaskCancellationShield (the proposal still uses that name in a few places). What prompted the change?

5 Likes

How about withTaskCancellationForceField?

Kidding, I like the ā€œshield terminologyā€, I think it’s appropriate and the metaphor works for me. Agree with the reasons mentioned above as to why ā€œignoredā€ is not the appropriate term.

In terms of the question of hasTaskCancellationShield (note the proposal mentions both hasActiveTaskCancellationShield and hasTaskCancellationShield), I agree with the notion that it’s not inherently unsafe, so why is it on UnsafeTask. But it does seem helpful as a property and maybe UnsafeTask is the most likely place for it to land.

The idea is to provide users with the ability to detect whether a cancellation shield is active for debugging purposes and to discourage its use in ā€œnormal codeā€. Think: ā€œif the shield is active, take this code pathā€ patterns.

I think we may need some more focused discussion about whether this API is worth including.

It sounds like Konrad, as the author, is pretty strongly in favor of including it for debugging purposes. Speaking just for myself, my inclination is that I think that there should be some kind of interface for getting at private task status information for debugging purposes — information like whether the task is cancelled, certainly, but also whether it's completed, whether it's currently running, what it's waiting for, and so on — but it's fine if that's unstable and doesn't make it easy to extract out specific details programmatically. And I'm not sure that just putting things on UnsafeCurrentTask does that effectively.

On the other hand, I can certainly imagine situations where it'd be useful to know that a task is running within a shield, not because you want your observable behavior to be different, but just because you know you can avoid certain overheads. For example, if you're waiting on a timer while running within a cancellation shield, you can avoid all the added overhead around timer cancellation.

4 Likes

In my opinion, for the stated reason in the proposal, it is worth having some form of debugging feedback on whether a shield is active. Exposing such feedback only through a debug string is misguided. Providing an API for this purpose is the correct direction. However, exposing it on the most accessible part of Swift concurrency, Task, will eventually lead to it being incorporated into some implementations workings (having an API intended solely for debugging on Task, in general, is not a good idea, in my opinion). For me, the question is less about whether we should have such an API and more about what its purpose should be. Answering this question will also guide us where such an API should be placed.

This question might be better addressed more broadly, as there's a lot of debug or test-only functionality it would be nice to have in the concurrency system beyond detecting shields. Perhaps, like @NotTheNHK suggests, debugging and test tooling could live outside the currency types developers use everyday, in a dedicated debugging or testing system. If that's the case, a shield API could live alongside others.

3 Likes

What about a TaskDebugInfo type to contain this debug info? It could then be available as a debugInfo property or TaskDebugInfo(for: Task) constructor (cf Mirror).

This feels like a compelling, non-debug use case for this API.

1 Like

Hi everyone, I’ve had sick week and still recovering to be honest but wanted to give a quick reply to the concerns the review posted here.

Yes, bottom line is that as we all know concurrency can be pretty confusing to work with, and we’re adding a new semantic here that can be pretty confusing unless it is possible to observe.

As someone who helps developers a lot understand some ā€œweird behaviorā€ it is, in my personal opinion, not negotiable if we should expose some way to query this behavior – we must allow querying this flag. Otherwise we’ll be stuck with confused developers unable to explain why ā€œsomething is stuckā€.

I do see the point that it’s somewhat unfortunate that there’s no better place to put these things, but currently Swift Concurrency mostly relies on Task to query things about the task so it is the natural place to put it.

If we were to invent some new place to put it, I think we’ll have to design a whole new system or family of APIs for all potential task queries, including perhaps ā€œwhat is the current executorā€ (which is debatable), and things like ā€œcurrent task idā€ and other things we’ve not surfaced currently. These things would not necessarily be just for debugging purposes, so it would have to be some new concept where we can get task information. I don’t know if this should take some key-value shape (task[.hasTaskCancellationShield]and task[.id] etc…) or just be a new type that we can query (like TaskInfo.current.something which tbh is a bit weird).

So unless we design, and agree on, some completely new mechanism for task information as part of this development cycle, I don’t think it is advisable to just not include the querying feature because it will cause lots of pain debugging systems as people adopt this feature without fully getting its implications – and developers will be stuck helping out, because they won’t have any way to diagnose and help resolve issues relating to cancellation.

Therefore I’d propose to accept this as is, and if we truly want to explore new ways and expose more task information, this feels to me like a future direction that will take quite some effort to design, but we could pursue it perhaps.

3 Likes

I fully agree that we should have this API, but I strongly believe that putting it on Task is the wrong direction if it’s intended for debugging. While the idea of a TaskInfo, DebugTask Task.info, etc. API is interesting and worth further exploration, it is clearly outside the scope of this proposal. If we don’t plan to implement such an API alongside this one, my opinion remains that we should put hasActiveTaskCancellationShield on UnsafeCurrentTask, as the original proposal suggested.

If the answer to my earlier stated question is that hasActiveTaskCancellationShield should be viewed as a ā€œproduction codeā€ API, then I don’t see a problem with putting it on Task.

P.S. If someone (including myself) wants to start a separate thread to discuss such a potential task-info system, I’m all for it.

2 Likes

SE-0504 has been accepted; please read the announcement for more detail.

I'd like to thank everyone who read and participated in this review for their contributions to Swift.

John McCall
Review Manager

2 Likes