Why `Task()` and Various Other Questions

Just when I thought I'd understood Swift Concurrency fairly well, I came up with an example project that made me question my knowledge of several of its mechanics.

Given the following ViewModel that is observed by a SwiftUI View, and compiled with Swift 6:

protocol ContentViewModel {
    var content: String { get }
    var buttonTitle: String { get }
    var showLoadingIndicator: Bool { get }
    @MainActor func onButtonPress() async throws
}

@Observable final class DefaultContentViewModel: ContentViewModel {
    let interactor: any ContentInteractor
    
    var content: String = "Lorem ipsum."
    let buttonTitle: String = "Perform Something"
    
    var showLoadingIndicator: Bool = false
            
    init(interactor: any ContentInteractor) {
        self.interactor = interactor
        NSLog("DefaultContentViewModel initialized.")
    }
    
    func onButtonPress() async throws {
        showLoadingIndicator = true
        
        Task { [weak self] in   // 1, 2, 3
            guard let content = self?.content else { return }
            self?.content = try await self?.interactor.loadContent(for: content) ?? ""
            self?.showLoadingIndicator = false
        }
    }
}

I am not able to explain these points:

  1. Does a weak reference to self actually avoid unwanted behavior here? I have been seeing this a lot in code recently. However, my vague understanding of the documentation is that this is unnecessary.
  2. The wrapping Task fixes the error "Sending 'self.interactor' risks causing data races". This is the main part that I do not understand. Doesn't the Task use the same concurrency context as the async function that creates it?
  3. How exactly is this different to a detached Task and why are both an option?

I believe my main source of confusion is how the word "context" is thrown around in discussions surrounding Swift Concurrency - by myself included.

What is the unwanted behavior you are referring to? If you worry about reference cycles, then adding weak for Task isn’t necessary and can be dropped.

This is a bit weird construct you have here, to be honest. Wrapping in a task make the function not to require async, but introduces unstructured task into the code, which in general can be considered as undesirable — you loose cancellation and errors propagation.

For why it resolves a warning/error on interactor call I believe this is a bug in compiler diagnostics.

To resolve the error you need to make interactor type either itself isolated or Sendable. Otherwise compiler will warn about unsafe (which it is) call to a nonisolated method from main actor isolation.

Detached Task is truly detached from the current execution, it does not inherit priority and task-local storage, in general you better to avoid it. There was nice comparison in another thread to similar DISPATCH_BLOCK_DETACHED of which many is just unaware (myself included).

1 Like

@vns I am not sure, but I think the Task resolves the error because it deduces (correctly?) that interactor is captured as a sending parameter.
I have not a good grasp on this new way to pass non-Sendable objects over isolation boundaries, but if I write a helper function to unbox the existential I can explicitly see the difference the sending keyword makes:

@MainActor
private func helper(theInteractor: sending some ContentInteractor) async throws {
    content = try await theInteractor.loadContent(for: content)
}

func onButtonPress() async throws {
    showLoadingIndicator = true
    try await helper(theInteractor: interactor)
    showLoadingIndicator = true
}

Deleting the sending keyword from the helper function results in the same error being shown (in helper itself, obviously), but with it the error is resolved. I think this is correct and how it's supposed to be resolved, but I am not sure.

@easyTarget I don't think there is a way to avoid using a helper function to properly denote the interactor as a sending parameter, if this is even correct.
I do think it is correct, though, because data-race safety is still guaranteed if I see this correctly: the interactor is not mutated while it is "stuck" in its loadContent function from what I can see here (it of course depends on your real, full implementation if these types, I guess).

If that is not the correct way, you might want to specifically enforce that your ContentInteractor protocol extends Sendable (this is not imolicitly the case!).

2 Likes

Reference cycles are one of them, the other issue being long-running tasks continuing after the caller, the ViewModel in this case, has been deallocated.

Is there another way to do this on a protocol level, besides making it conform to Sendable? After trying this out, this seems to be the much cleaner solution, and it matches what @Gero suggested.

To clarify, the idea is to generally implement ContentInteractor in Actors who e.g. access a database or perform network calls.

FWIW, actors are already Sendable so extending the ContentInteractor definition to protocol ContentInteractor: Sendable would probably not complicate things for you and also resolve the issue (and here I am confident that protects you from data races, as said I am not yet totally sure I understand sending).

1 Like

I was writing a long thoughts on why I think this shouldn't work, when suddenly realised: it is correct! At least, from the compiler perspective on this part, I guess.

Why this works, a bit of expansion: the type DefaultContentViewModel not Sendable, therefore it cannot leave its isolation, therefore it cannot be called in unsafe way which may caused data races, and compiler rightfully (but a bit confusingly to me) allows this.

Yet this makes me wonder: how that would be on the call site? We shouldn't be able to call this from a Task, right? Well, turns out we can:

extension DefaultContentViewModel {
    nonisolated func anotherCall() async throws {
        _ = try await interactor.loadContent(for: "")
    }
}

let vm = DefaultContentViewModel(interactor: DefaultInteractor())
Task { 
    try await vm.onButtonPress() 
}
try await vm.anotherCall()

But is this safe? To me there is a possible data race, because onButtonPress is going to be called from main actor isolation, and anotherCall from no isolation, and interactor neither Sendable nor isolated. So this feels like something here is missing: either my understanding or compiler diagnostics :slight_smile:

This looks like a compiler bug / regression specific to Tasks, a similar case has been reported already: Able to call `@MainActor` isolated function from non-isolated context in Swift 6 language mode · Issue #76649 · swiftlang/swift · GitHub

3 Likes

As Task is unstructured, you have to handle its cancellation manually. But as for cycles, there is no need to worry – Task will handle this correctly on its own. As for larger scale, I would avoid using Task unless it is only way – like calling this inside button handler in UI part.

In theory, other options is to make protocol either isolated to a global actor or required to be an actor. But this is too much of a constraint as for me, so in this case adding Sendable to a protocol requirement would be a preferred way to go from my perspective.

1 Like

Yes, and this has been there for a while. For instance, I have stumbled across similar bugs in diagnostics: Confusing error with sending value (+ opaque return type) (there is a corresponding issue created as well).

Wait, now I am confused... is this a problem with the original code (using a Task to capture the interactor as sending parameter) or my proposed unboxing helper function (with an implicit sending)?

Because if I play around with the code that calls an instance of DefaultContentViewModel (slightly modified, see below) I do get a compiler error (though a weird one) in Swift-6-language-mode.

I slightly modified your code to run in a command line app (where the top level code would be isolated on the main actor):

Task.detached { // ensure we're not on the main actor so the inner task does not inherit it
    let vm = DefaultContentViewModel(interactor: DefaultInteractor())

    Task { // no error in Swift 5 mode, but error in Swift 6 mode!
        try await vm.onButtonPress()
    }
    try await vm.anotherCall()
}

The error in Swift 6 mode is

Value of non-Sendable type '@isolated(any) @async @callee_guaranteed @substituted <τ_0_0> () -> (@out τ_0_0, @error any Error) for <()>' accessed after being transferred; later accesses could race

which, ignoring the not-exactly-beginner-friendly barrage of annotations, seems to go in the right direction. The quickfix (well, not a fix in this case, but still) says "Access can happen concurrently" and provides a "Show" button which then highlights the anotherCall function (which is the problematic part).
A secondary error in the next line also gives the more obvious

Sending 'vm' risks causing data races

That is to be expected as the inner Task inherits the isolation of the outer, i.e. is not running on the main actor, so vm would need to be sent (and since apparently it's accessed later the resulting implied capture parameter cannot be sending). At least that's how interpret it all together.

Surprisingly, if you naively try to annotate the inner Task with @MainActor inside its closure to address at least the second error (and thus basically arrive at your example, @vns), it all changes to one error on the Task line:

Pattern that the region based isolation checker does not understand how to check. Please file a bug

:laughing: ... but only if you leave the call to onButtonPressed in it, so the compiler still "somehow" gets that this is not possible (just the error message falls over).

Lastly, if you remove the offending last call in the outer Task that calls anotherCall it works, as there are no more subsequent calls to vm from another context, so it can be passed as sending.

All in all this seems to be a slightly different bug in mode 5 than the one @nkbelov linked, no?

Either, any version of the class in nonisolated form has this issue. TBH I am still sort of confused about interactor being able to treated as sending – it is a part of a class, which is then can be part of any isolation/task, and therefore should be bounded to the same region as holding type.

My turn to be confused, probably? :slight_smile:

Yeah, but why is detached needed to get an error? This line:

try await vm.anotherCall()

in my version gets vm to be sent off the actor, causing the same issue (at least to my understanding).

And I guess this actually answers this question – Swift just cannot diagnose these whole thing, your tricky version seems to surface missing diagnostic.

I feel like all these bugs are connected, as they either not treating sending correctly or skipping checks inside Task.

1 Like

I ran the code in a simple command line app (using the default Xcode template, which includes a main.swift file for the code). The top level code thus runs on the main actor and I wanted to ensure that there is an isolation context switch in there somewhere. I understood your code snippet as meaning:
"vm is instantiated outside of any actor isolation, then we start a Task to call onButtonPress. That call is isolated to @MainActor so would require a hop, but outside the Task the call to anotherCall happens, which is not isolated. That results in an uncaught race"

Just pasting your code into the template (on the top level) would not be enough as we start on code that runs on the main actor already. While anotherCall is not defined to enforce isolation to a specific actor (nonisolated), just calling it in that scope still runs it on the main actor[1]. Even though that happens after starting a Task, which, without explicit annotation, also inherits the @MainActor isolation. This means all calls run on the main actor, there is no race, and the compiler correctly does not complain.
Again, that is just due to the fact the sample app I used starts on the main actor.

EDIT: I am an idiot... I link SE-0388 below, but forgot it affects this specific place...
You're right, the call to anotherCall should emit a compiler error! I'll leave below parts in here as they show that weirdly enough, wrapping everything in a detached Task does correctly warn you...

To reproduce the issue you illustrated I wrapped your code snippet into a detached Task, that allows me to basically "escape" the main actor. Now vm is no longer instantiated on the main actor and the call to anotherCall no longer runs on the main actor either (just like in your snippet).
If we were to just call vm.onButtonPress()[2] we mess up: We send vm to a different isolation context, main actor, but it's still used afterwards and cannot be treated as sending. Now that I look at it again, it actually doesn't matter whether we put the vm.onButtonPressed() call into a Task (another one inside my detached one), the underlying problem is the same.

I think this is basically exactly what you showed in your snippet, but as said, I do see the compiler complain, at least in Swift6 mode (and btw, now that I checked again, also in Swift5 mode with anything but minimal concurrency checking).

I'm not sure I get you right here, but I think that's not the exact issue. The function is nonisolated, yes, and due to SE-0338 (and before any changes to that) it means it hops off the current actor (thus sending vm to a different isolation context), but that is only a problem if any other calls would still be able to use vm after the hop happened.
Ultimately that's not different from the hop vm needs to do for onButtonPress that is needed when vm wasn't instantiated on the main actor in the first place: Any subsequent references to it on the original isolation context prevent it from being treated as sending, so the compiler complains.

I like to think of sending as the "weaker Sendable": You can "send" non-Sendable objects over to another isolation context, you just have to ensure that nothing can mutate them on the original context afterwards.
In our sample code that means it is the conjunction of onButtonPressed and anotherCall that results in an error.
There may be more, but I don't see any uncaught race-conditions myself. That doesn't mean much as I am relatively new to sending...

Definitely, though in our case I think the result is not as severe as you get at least some error that prevents you from compiling a data-race (again, unless I missed something...)


  1. I am an idiot... see below ↩︎

  2. and vm.anotherCall() after that! ↩︎

1 Like

I think we on the same page in general, I just not fully understand compilers reasoning about isolations...

OK, I was puzzled a bit, thinking that probably I got race conditions and sendability in that case wrong, but then I've found a new way to overcome the compiler errors in a weird way.

Let's emulate calls from UI in some way, and declare a struct:

@MainActor
struct EmulatedView {
    let vm = DefaultContentViewModel(interactor: DefaultInteractor())
}

We can say that vm here is in main actor region in terms of RBI. Now, let's try to provide some calls:

extension EmulatedView {
    func handlerA() {
        Task {
            try await vm.onButtonPress()
        }
    }
}

This works fine as it should: nothing getting sent nowhere, everything stays on the main actor. Seems about right. What about second nonisolated call?

extension EmulatedView {
    func handlerB() {
        Task {
            try await vm.anotherCall()  // error: sending `vm` risks causing data races
        }
    }
}

Compiler, as expected, disallowed this for us. vm cannot be sent (as it is not sending) because it belongs to main actor region (hope I'm getting everything right :sweat_smile:). And there is a compiler note about that:

note: sending main actor-isolated 'self.vm' to nonisolated instance method 'anotherAccess()' risks causing data races between nonisolated and main actor-isolated uses

But now we try this little trick with sending you came up:

extension EmulatedView {
    func helper(vm: sending DefaultContentViewModel) async throws {
        try await vm.anotherCall()
    }
}

and update handlerB relatively:

extension EmulatedView {
    func handlerB() {
        Task {
            try await helper(vm: vm)  // ok!
        }
    }
}

Error is gone, but why? vm was isolated to main actor and remained there. If it was indeed sending, then compiler should've allowed just bare use, not go with the wrapper trick.

We can then define a few more nonisolated methods on the DefaultContentViewModel, call them through a wrapper in other handlers, and call all of them in a row. Without helper trick, Swift of course will disallow this – and this is a direct impact of SE-0338 and later RBI diagnostics. So the fact that helper can be called in a such manner seems not right.

1 Like

Oh, for sure we are! And I'm in the same boat, as I said, the relatively new sending is definitely something that I do not fully understand.

This is exactly what I don't get either. I am not sure whether it's a bug (i.e. there should be a compiler error) or something I just don't understand.
From what I understood about sending so far (and I think that's at least one usage) it's basically equivalent to consuming, just in regard to isolation context. That would mean if you create an instance of some not-Sendable type in one context, maybe work with it, but then hand it over to another context, you can do that, but the compiler ensures that you for sure won't use it in the originating context.
For the simple case, where the variable holding the instance goes out of scope some time after it was passed over that's a simple check: do you do anything with it after the handing over, before the scope ends? If no, everything's fine, if yes, error. That's what you can see happening in my example with the detached task.

If you keep a reference of the instance around, though e.g. in a property like in your latest example, I don't see how that's not an issue. I think the compiler should emit an error on try await helper(vm: vm), something like "passing vm as a sending parameter when still holding onto it risks data-race" or the like...
And I don't know if that fits in the bug repors we already have either... :sob:

1 Like