Lately having headaches being not able to async defer, so for sure +1 from me.
Agree with @grynspan and @FranzBusch that throwing is also needed, we basically have just async and throws to control effects, and not having one of them sometimes challenging.
That said, moving forward step by step is a good approach, and I’m looking forward for implementation. 
1 Like
-
Yes, they can observe
Task.isCancelled == truejust as they can today!
I do think we need to provide a solution for teardown code to run in an un-cancelled task.
The reason for this is that many async functions won’t do their work if they’re started on a task that’s already cancelled (and usually throw CancellationError). One example is any use of Task.sleep will throw CancellationError.
Now what if you need to call a function in your teardown procedure that doesn’t operate normally during teardown?
In all my code I use the asyncDo:finally (async-http-client/Sources/AsyncHTTPClient/StructuredConcurrencyHelpers.swift at 254d34096123cd5da4f82e69f52f622ac7526d68 · swift-server/async-http-client · GitHub) helper which runs the finally block (which is the same as defer) un an uncancelled task. I’ve seen way too many issues where the teardown didn’t happen properly and ended up failing because the task was pre-cancelled.
The other reason is: What if you need to do a teardown action that you want to set a timeout on? If the task is already cancelled when you enter the teardown block, you’ll have a very bad time because you can’t “double cancel” it but cancellation is the mechanism that’s typically used for timeouts.
Hence, I really think that async defers need to be run in an uncancelled task or else we’ll see tons of bugs. I’m also happy if we give the user control over the ‘inheritance’ of the cancellation flag into the async defers but the default should be “not cancelled” and not “inherit cancellation”. I’ve extensive experience in very-large scale systems with asyncDo:finally (and asyncDefer:after another helper just like asyncDo:finally but with the arguments flipped).
6 Likes
Also non-async functions may just be doing if Task.isCancelled return though.
So there’s two features to talk about here, and deciding what behavior of a defer is, because the cancellation specific to async functions at all actually.
The problem you mention is real and I agree we need this shielding behavior; To clarify for readers of the thread, it should not be done by creating new tasks though as your existing workaround helper does today though. I know we both understand that but let me write this here for the thread and reviewers. I basically think we need:
Task.isCancelled // true
withCancellationIgnored {
Task.isCancelled // false
}
which works by inserting a record into current task to handle this.
The question then becomes, if this is just defer that allows async code; it probably should not shield/ignore by default, since it also doesn’t today for normal synchronous code (!). So we might therefore end up with…
defer {
withCancellationIgnored { // hm, verbose (-1), but composes (+1)
cleanup()
}
}
is perhaps a bit too annoying, so I wonder if we’d end up with both: withCancellationIgnored and some form of shorthand defer(ignoreCancellation??) {} (ugh bad name… defer(???))?
You might want to run code “always” even outside a defer so I think we want that withCancellationIgnored feature anyway and it’s separate from defer.
6 Likes
Thanks for this (and thanks to @FranzBusch for similar commentary). I'm of two minds here:
- On the one hand, allowing
awaits insidedeferdoes not enable any behavior that is meaningfully different from manually invoking cleanup along each exit path 'manually'. IMO this pitch is a strict improvement over such approaches—it doesn't introduce any new behavior with respect to cancellation, and makes it much harder to forget necessary cleanup. - OTOH, for folks who work around this limitation today by slapping their cleanup in a
Task { ... }the picture is more muddied. While this has the typical issues associated with kicking off a (throwaway) top-level task (e.g. resource overhead, cleanup runs at unspecified point in the future), you make a good point that it is hard to claim strict improvement here. Perhaps users are better off reflexively wrapping inTask { ... }and if allowingawaitinside defer becomes the path of least resistance, we'll invite people to ignore cancellation.
I think I still come down on the side of this being a step in the right direction for a couple reasons:
- We still get the benefit of being able to direct the folks from (1) above towards
defer, which I think is a big win. - To the extent most cases today that we'd want to worry about are throwing functions (which throw/rethrow a
CancellationError), users will still get 'notified' of this when they try to writetry await ...in theirdeferand this fails to compile, at which point they'll have to reckon with the potential error scenarios. - As @ktoso notes, this problem already exists for synchronous code (though I expect it is less common).
Also, while I understand the problem you raise in theory, it would definitely be helpful to see examples of problematic cleanup methods of the type you're describing. While, yes, Task.sleep will throw immediately if cancelled already, I don't think that's particularly compelling as an example since its not something that's doing necessary cleanup. It strikes me as pretty surprising library design if a method which performs some Very Important Cleanup might actually skip that cleanup just because the current task happens to be cancelled. Which isn't to say we shouldn't have something like withCancellationIgnored to support such cases, but it makes me less inclined to think that suppressing cancellation in defer ought to be the default.
6 Likes
While, yes,
Task.sleepwill throw immediately if cancelled already, I don't think that's particularly compelling as an example since its not something that's doing necessary cleanup
Task.sleep is the building block for anything with timeouts. HTTP requests are a common thing requiring timeouts –> bad.
It strikes me as pretty surprising library design if a method which performs some Very Important Cleanup might actually skip that cleanup just because the current task happens to be cancelled.
A function like a HTTP request, a file system operation or something isn’t designed as “Very Important Cleanup”, it’s designed as a general-purpose library. However, it’s often used in cleanup.
The first issue I’ve seen was a library equivalent of rm -rf in a file system library. It didn’t perform the removals if the task was already cancelled. That’s kinda expected because a deep rm -rf can take a long time. So it makes sense that you could interrupt it by cancelling it. However, if you need it for cleanups (very common), you really want it to perform work.
Other times when I’ve seen this exact issue:
- Subprocess spawning to run stuff to clean up
- HTTP requests for cleanups (usually destroying some service resource)
- File system removals
- Miscellaneous functionality that (correctly) uses
Task.sleepor anything like it in its implementation and just happens to stop to work
What’s worse is that adding a Task.sleep (or any other function that doesn’t work normally on already-cancelled tasks) into a miscellaneous library function is hardly considered a breaking change. But unless you run cleanups in non-cancelled tasks, this will be breaking. I’ve been debugging these issues for years now and the only fix that worked is to default to uncancelled tasks for cleanups.
8 Likes
That’s all valid points Johannes.
I am concerned about an nuanced difference in code making the difference in suppressing cancellation or not, when moving between synchronous and not synchronous code:
func cleanup() { guard !Task.isCancelled else { return } }
Task.isCancelled // true
defer {
Task.isCancelled // false
cleanup() // OK, runs!
await whateverSomeAsyncCleanupOrAnythingElse()
}
defer {
Task.isCancelled // true
cleanup() // whoops, NO CLEANUP!
}
You are right that this is more problematic for async code, but it definitely is not limited to them! There can be plenty synchronous “cleanup” functions which check cancellation exactly because they are synchronous and “slow” so they’ll try to not run for some reason when cancelled. It very much is an unfortunate composition problem.
I think we should avoid the above situation where some arbitrary, perhaps even unrelated, await will make your cleanups work or not. I’m not opposed to updating defer behavior, but it’d be good if we either find an additional spelling or some other way to make it consistent, regardless if there was an await or not in the defer block. It could be an option in defer, or some other word instead of defer (that may be a high ask though, but it is an option).
4 Likes
There can be plenty synchronous “cleanup” functions which check cancellation exactly because they are synchronous and “slow” so they’ll try to not run for some reason when cancelled.
Yes, valid points. I will say that I have personally not come across a synchronous function that checks for cancellation and often when they’re “slow”, they’re also blocking. And when they’re blocking you’ll likely want to offload them from the Swift Concurrency thread pools and outside of the Swift Concurrency thread pools there’s no concept of cancellation.
But yes, it’s a difficult composition problem unfortunately.
3 Likes
Maybe that could be made to work at the expression level similar to try and await:
defer {
uncancellable cleanup()
}
3 Likes
Ignoring defer bodies for the moment, what is the general guidance for running asynchronous work even if canceled? My understanding (echoed by the asyncDo example up-thread) is that if you want to opt-out of cancelation you make an unstructured task.
Given this, why not just say that if you want to do cancelation ignoring async work in a defer then you need to wrap it in an unstructured task:
defer {
await Task { /* ... */ }.value
}
This is effectively the same shape as what the asyncDo is doing. It has the benefit of being caller defined.
I would have reached or something like this in async defer bodies out of habit. I’m wondering if the habit is a good idea. Why do we need a dedicated withCancellationIgnored construct?
4 Likes
Throwing two more of my cents in the ring: it would violate the Principle of Least Surprise for defer to sometimes not run because the current task has been cancelled. That behavior is not observed for synchronous defer, so why would it suddenly occur if I add an await expression in the middle of my existing defer block?
Of course, if I write:
/* async */ defer {
await withTaskGroup { taskGroup in
taskGroup.addTaskUnlessCancelled { ... }
}
}
Then the task in the task group won't run if my task has been cancelled, and I've opted into that behavior the same way I would elsewhere in my code.
10 Likes
I don’t think anyone is suggesting behavior which would simply not run defer bodies in a cancelled task (that certainly not my suggestion). The question is whether the defer body will inherit the cancellation of its containing task or have that cancellation ‘hidden’ from it for all downstream calls.
To approach this a bit more systematically I see basically 4 ways that cleanup work could conceivably interact with task cancellation:
- No interaction. The cleanup operation runs regardless of cancellation.
- Skipped but necessary. The cleanup operation is sensitive to cancellation, but it must be performed and so clients must somehow get it to run in an uncancelled task.
- Skipped and unnecessary. The cleanup operation is sensitive to cancellation because it knows its operating on somehow task-bound resources so if the task is cancelled the operation doesn’t need to run twice.
- Skipped and unnecessary, but can only run once. The cleanup operation skips itself in a cancelled task because the cleanup will have already run (or will run soon) by virtue of the task cancellation, and it is an error to run that cleanup twice.
I personally most commonly encounter the desire for async defer when dealing with (1). E.g. the cleanup is synchronous but happens on another actor, or the cleanup operation ‘knows’ that it needs to run to completion always and so never checks cancellation.
(2) is the problematic case for a naive async defer which makes no effort to adjust cancellation. We might cause users to inadvertently skip cleanup because they think “defer always runs” and don’t pause to think about task cancellation.
(3) and (4) are variants which are problematic for a rule of “deinits are always uncancelled“. (3) is perhaps just inefficient, and (4) would be very similar to (2) in terms of being a proper bug in the program. I am not really familiar with operations that fall into this bucket, but it doesn’t totally strain credulity to me to think they might exist.
I think there’s a colorable argument that (2) is sufficiently common and sufficiently problematic that it’s worth taking seriously the suggestion that we avoid making the default async defer behavior “cancellation matches the containing task”. I’m not totally convinced that’s the correct decision, but if we do reject the naive option, what options are we left with?
- Pick the default behavior of “cancellation is suppressed within
defer. This would spell trouble for case (4) above since such code run in adefer(thinking “it knows the right thing to do with cancellation”) would accidentally introduce bugs. Moreover, I think we would want/need a way to opt out of cancellation suppression: while one can write aTask { … }today as a cancellation guard, I can’t think of a mechanism by which a defer body could recover the enclosing task’s cancellation if we decide to suppress it by default. I’m also open to hearing an argument that cases (4) (and (3)) are so exotic that they do not need special consideration at this point. Also, as has been noted up-thread, we’ve already missed the boat on this for synchronousdeferwhich leaves us with two sub-options:
a. Live with this inconsistency indefinitely and accept that there will be semantic differences between synchronous and asynchronous defer bodies.
b. Attempt to change the behavior for synchronousdeferbodies, likely in a future language mode. - No default. Rather than just adding
awaitto turn adeferbody asynchronous, one must choose between the variants (e.g.defer(inheritCancellation)ordefer(suppressCancellation)). This is the most conservative option in terms of giving us room to decide on the default behavior separately from introducing the underlying functionality. - Inherit cancellation but try to mitigate. E.g., we could disallow the use of
try?within asyncdeferbodies and use that as a ‘hook’ to inform users about the potential issue. We could provide a diagnostic like “to ignore errors within an async defer body, useignoreCancellation { … }”.
IMO having to deal with synchronous defer puts (1) in the ‘not worth it’ camp—I think having behavioral differences introduced by adding an await is not a good ‘resting place’ for the language (ruling out (1)(a)), and I would definitely need to be convinced that the potential behavior change for synchronous defer is sufficiently benign to ship even behind a language mode—while I would think that most cancellation-sensitive defer bodies should sufficiently handle the case where they are suddenly running ‘uncancelled’, it’s a very subtle change that makes me a bit queasy.
If we can’t come to consensus on a reasonable default behavior then (2) at least provides a path forward. It perhaps harms progressive disclosure since suddenly just moving some work to a different actor and adding an await forces you to reckon with this subtle task cancellation nuance. But if we think that these cases are problematic enough to warrant having no default behavior, perhaps that’s exactly the right point for users to confront this question!
(3) doesn’t really feel like a viable option to me either. It feels like it makes the language messier in a way that is much better suited by solution (2).
I don’t think I’m swayed from my position that “inherit cancellation by default” is the right choice but I’ll make sure to add this to Alternatives Considered. My preferred alternative, if we do decide cancellation inheritance is too problematic, would be (2) above (if we can pick a spelling that is not too obnoxious). I would really want to avoid a behavior change for async defer bodies which is not very explicitly opted into. Adding await or even requiring defer async doesn’t communicate this well enough, IMO.
6 Likes
I think blocking cancellation is just a custom case. In more general case we want to have customizable cancellation policy.
So the keyword/API for this should return/accept cancellation token that could be used to cancel cleanup activities on a different policy, e.g. by a timeout.
1 Like
This is the “workaround”, the same way Jonannes’s current day “asyncDo” or others are doing. It is not optimal because you need to actually schedule this new task which can lead to arbitrary delays for the cleanup.
If we were to run in the same task but ignoring the cancellation flag, this allows us to execute the cleanups immediately, without being at the mercy of scheduling luck – which may be quick, or may be very slow on a loaded system. Especially for cleanups you’d want them to happen quickly, so I do think we do need the “ignoring cancellation” semantic within the current task.
1 Like
I generally agree with the analysis of the 3 last cases there.
(1) The arbitrary inconsistency because “there happened to be an await somewhere” suddenly causing cancellation behavior differences isn’t a good place to arrive at design wise; it’s too arbitrary and would be bound to show up in some talk titled “101 swift gotchas!” in the future… 
(1.b) so changing the default of existing defer might be worth considering but it’s also pretty hidden which is worrying about it, if we did the behavior change under a language mode. However, IF we decide that defer are the way to do the cleanups it might be worth considering.
I’ll get back to this one below.
(2) It’s a bit hard to call this “no default”, but I think you mean “when defer is asynchronous, you have to choose” (therefore “no default” but “specifically in asynchronous defer case”). It is most conservative but also quite messy and does further complicate the usually simple defer logic into something much more complicated that people would have to learn immediately. So I’m not too convinced about it.
Especially since this would likely end up as synchronous defer needs this too because it would be weird for only async defer being able (being forced to choose), so a normal defer should at least be able to set the same…
I think (2) is worthwhile but as opt-in, both for synchronous and asynchronous defer, once we do have some way to suppressCancellation { … } and it’d just be a short hand for it, to avoid double nesting `defer { suppressCancellation { } } → defer(suppressCancellation) {}could be nice… but is just sugar, so we may wanna be careful about too much sugar.
(3) Is an interesting idea but feels somewhat arbitrary… I don’t think it actually solves the problem, but just would be more annoying without really explaining to developers “why” a defer block with async code is annoying them more than normal code 
Back to the core question though… I think there’s a few steps here to get to what @johannesweiss and anyone who actually wants to do resource management actually want:
Step 1) I personally think “async defer” should be just that “a defer that can do async” and change absolutely nothing about cancellation behavior.
Step 2) We need a simple feature to ignore cancellation within a task, we can bikeshed the name, but it’d be some form of withTaskCancellation(.suppressed) { ... } or similar (probably with…).
Step 3) The 
actual solution to resource management which I believe has to do more than just defer, and it’d require cooperation of the resource:
// some future direction; Horrible names on purpose;
// I don't want to discuss the exact details of this in THIS thread.
protocol ManagedResource {
func beginResourceAcccess() async throws
func endResourceAccess(failure: Error?) async throws
}
struct MyFile: ManagedResource {
func beginResourceAcccess() async throws { open file }
func endResourceAccess(failure: Error?) async throws { close file }
}
func use() async throws {
with let file = MyFile() // NOT ACTUAL PROPOSED SYNTAX; something like it tho
try ... // throws trigger the end resource with error
// end of scope triggers end resource access; it may suspend or throw
}
This is similar to Java’s try-with-resources (try (var file = open() {}), Python’s context managers (with open('some_file', 'w') as opened_file:) etc. I think this is the actual thing we want here, and it should be a tool that helps us get rid of the very complicated with…-style methods we have everywhere nowadays. The resource managed object could shield cancellation or not, and it’s nicely centralized; not up to the user of the resource at all, which is what we want.
It is much more work than just an asynchronous defer, so I’m very keen on keeping those separate, and separated out in those 3 steps.
I think we can do (1) in this proposal; we can do (2) very quickly in a follow up proposal. Giving us a verbose, but correct way to handle the problem. And then we should design (3) which is the “real solution” which would subsume the use of defer for “resource management”.
5 Likes
This is a great, and overdue, discussion about async resource cleanup. I'd just like to chime in on the original thread to say I'm in support of just adding async defer first, leaving throwing and cancelation shielding as follow-on proposals. Even with the manual cancelation shield using await Task {}.valuethis is still an improvement, as you can now put the shield inside the defer block, rather than creating a nested scope for the entire cleanup logic.
For the overall async resource cleanup story, I wonder if a mini-vision should be written up, which would reference and help drive these incremental improvements one by one. But that'd still allow this proposal to move forward without gaining more scope.
9 Likes
I’m very much in favor of some sort of withCancellationIgnored() helper.
But also, this statement made me wonder if Task.immediate might help with the scheduling issue.
If the async defer runs in the same isolation as the body, it seems like the task should be able to start synchronously, without suspension or scheduling delay. (I don’t have much experience with Task.immediate, so please correct me if I'm wrong.)
e.g.
func f() async {
defer {
await Task.immediate {
// - shielded from outer cancellation
// - starts synchronously
// - completes as part of `defer`
}.value
}
}
4 Likes
+1 for the feature – thanks for pushing it! i think the proposed decisions are the right ones as i feel they extend support for async expression handling to defer statements in the most 'natural' way[1]. like others have expressed earlier, i also think the questions around 'cancellation shields' and error handling are separable and shouldn't block the proposed improvement here.
thanks for explicitly stating this – i also see that Holly drew attention to it in the current implementation PR, but i wanted to reiterate that we should try to avoid inadvertently introducing more surface area for surprise isolation crossings; e.g. like the one withTaskCancellationHandler currently has which can introduce suspension points due to its interaction with closure isolation inference rules. put another way, async defer bodies should definitely have nonisolated(nonsending) semantics.
if you were relatively new to swift, understood
deferandasyncthen i think this is exactly how you'd expect the composition of the features to behave. ↩︎
3 Likes
That’s a good point actually! I implemented that but forgot to mention in this context
Yeah that’s one suggestion for current-day until we have the async defer or the cancellation shielding 
1 Like
FWIW, the scheduling issue of Task { … }.value is not the main problem. The main problem is that this isn’t in the actual task tree. So if you’re looking at swift inspect dump-concurrency, you won’t see it in the correct place.
So if we decide (and I still think it’s a mistake to ask the user to start an un-cancelled child task) that it’d always be
async defer {
try await withUncancelledTask {
try await cleanup()
}
}
we’d still need to make sure that withUncancelledTask is not just Task/Task.immediate { … }.value but rather makes a child task that’s correctly nested (but guaranteed to not start pre-cancelled).
Why do I think it’s a mistake to default to “inherit” cancellation in defers? IMHO it’s a grave mistake because anything you put there may happen, may not happen or may partially happen. If it’ll happen depends on implementation details (what happens when run on an already-cancelled task) that are not typically documented of anything that’s called transitively. Worse, adding a try Task.checkCancellation() or a try await Task.sleepor anything else that chances the behaviour under cancellation is typically not considered a SemVer major change. So the behaviour may change at any point in time…
This implies that a programmer will need global reasoning to use async defer, IMHO completely against one of Swift’s core design principles: Local reasoning. The only way a programmer could correctly use async defer & rely on local reasoning with cancellation inheritance correctly is to always put a withUncancelledTask inside of it. If you don’t put it, you simply cannot know what happens unless you carefully analyse all transitively called code and redo that analysis after every swift package update or other code change. Also, you won’t be able to do anything with timeouts in there (because double-cancellation isn’t a thing) – this IMHO turns async deferinto something that’s simply not useful in any context.
I’d be happy if we did (pseudo-language) async defer(inheritCancallation: Bool = false) { … }. So if you write async defer { try await foo() }, foo() will be run an an un-cancelled child task. But if you write async defer(inheritCancellation: true) { try await foo() }, foo will run and it’s cancellation status would be inherited.
There’s a separate question to me whether defer should just allow awaits or whether we need async defer. One advantage of async defer is that we’d get to decide the semantics w.r.t. cancellation.
3 Likes
To be clear here: I already said the reason to build withUncancelledTask is to not have to start a new task, that’s well understood.
This Task.immediate is just a suggestion for the current workarounds that already create a new Task{}.
2 Likes