Suggestion - Runtime warnings in debug builds if @MainActor methods/vars are called from background threads

I thought it was your blog which quotes this line from Apple:

By adding the new @MainActor annotation to Photos, the compiler will guarantee that the properties and methods on Photos are only ever accessed from the main actor.

The quote is an example of the kind of description I see in less formal contexts.
In this case, it was a quote from an Apple rep in a WWDC talk.

I wouldn't call that a documented guarantee, and it was only intended as an example of what people describe/expect.

There is of course lots like that in tutorials/forums/etc - as well as in many other Apple talks.

For example in 'Eliminate data races using Swift Concurrency', there is a stronger quote:

Isolation to the main actor is expressed with the MainActor attribute.

This attribute can be applied to a function or closure to indicate that the code must run on the main actor.

Then, we say that this code is isolated to the main actor.

The Swift compiler will guarantee that main-actor-isolated code will only be executed on the main thread, using the same mechanism that ensures mutually exclusive access to other actors.

but again - I don't take WWDC talks as 'documentation'

The documentation you’re looking for here probably falls out of the proposal for actors (SE-0306) and global actors (SE-0316). I’m not sure you’ll find quite such a pithy quote , but SE-0316 does say this:

Note that the data in this view controller, as well as the method that performs the update of this data, is isolated to the @MainActor . That ensures that UI updates for this view controller only occur on the main thread, and any attempts to do otherwise will result in a compiler error.

And the proposals are broadly clear that, at least within Swift, actor isolation isn’t really ‘optional’—it should be that anything isolated to an actor (global or otherwise) cannot be synchronously accessed from outside that isolation domain.

One of the tests for the actor data race warning is also something that I'd expect to be a compile error as it's very straightforwardly doing something which violates actor isolation. The other one which uses an unsafeBitCast is reasonable though.

I figured I should stop complaining about the lack of documentation, and have a go myself at writing the @MainActor rules...

I couldn't find a bug report for this specific problem, so I went ahead and filed one: Using key path literals as functions defeats @MainActor checks · Issue #63189 · apple/swift · GitHub

I also agree that the other @MainActor loopholes @ConfusedVorlon identified are important to either close or document very explicitly. @ConfusedVorlon Thank you for your articles about these problems!

5 Likes

I found another 'loophole' today.

Binding captures getter/setter callbacks.
The compiler considers these as in the context where they are created (so you can call @MainActor in your setter if the binding is created in @MainActor)

But the actual callback runs merrily on whatever thread it is called from

That works as expected (you can return arbitrary objects from the MainActor annotated function, those objects could be called on arbitrary threads).

This doesn't work either but it is closer to what you need:

    // no @MainActor annotation here
    var binding: Binding<Bool> {
        Binding<Bool> { @MainActor in
            dispatchPrecondition(condition: .onQueue(.main))
            return true
        } set: { @MainActor _, _ in
            dispatchPrecondition(condition: .onQueue(.main))
        }
    }

I'd also recommend to send code snippets as text embedded in ``` brackets.

you might expect that @MainActor functions can be called on arbitrary threads.
(I know it to be true)

But that's explicitly what the compiler is trying to stop, and is also what many users think is guaranteed not to happen. (and indeed what Apple videos promise)

Given the almost complete lack of documentation around @MainActor - it's not very surprising that people don't know what it does.

Still - the fact that the compiler will object if the variable is not marked @MainActor even though it doesn't enforce the @MainActor-ness is at the very least misleading

struct ContentView: View {
    var body: some View {
        //Button just updates the binding when clicked
        //But on a background thread
        BoundButton(val:binding)
    }
    
    @MainActor
    var binding:Binding<Bool> {
        //The Binding is created on @MainActor
        return Binding<Bool> {
            return true
        } set: { _, _ in
            //But the setter could be called from any thread
            //So the compiler shouldn't allow this call to
            //@MainActor annotated shouldBeOnMain
            
            //Error generated here if variable is -not- marked @MainActor
            //Call to main actor-isolated instance method 'shouldBeOnMain()' in a synchronous nonisolated context
            shouldBeOnMain()
        }
    }


    
    @MainActor
    func shouldBeOnMain() {
        if !Thread.isMainThread {
            fatalError("Not on Main!")
        }
    }
}

struct BoundButton: View {
    @Binding var val:Bool
    
    var body:some View {
        Button {
            Task.detached {
                val = false
            }
        } label: {
            Text("TryMe")
        }
    }
}
1 Like

Try adding @MainActor annotation on the @Binding variable:

@MainActor @Binding var val: Bool

Note that the checks in the compiler are quite shallow:

DispatchQueue.main.async {
    val = true // ok
}

let queue = DispatchQueue.main
queue.async {
    val = true // not ok
}

I agree in that it's under documented and underperforming.

I think you're missing my point.

I'm not looking for coding help. I'm pointing out yet another way that @MainActor doesn't work in the way that is expected.

1 Like

I was merely pointing out that while it's far from ideal, at least this situation is handled correctly:

struct BoundButton: View {
    @MainActor @Binding var val: Bool
    
    var body:some View {
        Button {
            Task.detached {
                val = false // 🛑 Main actor-isolated property 'val' can not be mutated from a Sendable closure
                DispatchQueue.main.async {
                    val = false // ok
                }
            }
            Task.detached { @MainActor in
                val = false // ok
            }
        } label: {
            Text("TryMe")
        }
    }
}

I would have expected a compilation error in this code when compiling under strict concurrency checking (which will be enabled by default in Swift 6). However, it appears that even under strict concurrency checking, this isn't being diagnosed.

I would expect it because the parameters passed to Binding.init(get:set:) are not main-actor isolated. The set closure declared here seems to be inferring @MainActor isolation from its declaring context, but it is being passed off to Binding.init(), which doesn't promise to fulfill that contract.

Is it expected that a @MainActor closure can be passed to a non-actor-isolated argument?

1 Like

I think this is ‘okay’ because the closure is formed on the main actor and is not marked @Sendable so it Should :tm: be impossible for the formed function reference to be handed off to a different isolation context and executed there.

That that’s happening seems like a bug internal to SwiftUI to me but I don’t think it’s a problem in the client code, unless I’m missing something.

the promise of modern concurrency is that the compiler guarantees correctness (at least at some level)

this shouldn't depend on whether the framework you're passing to is buggy...

Having said that - there is no SwiftUI bug here. SwiftUI makes no promises - so we can't blame it for not keeping them!

public init(get: @escaping () -> Value, set: @escaping (Value) -> Void)

the compiler doesn't know anything about where this closure is going to be run.
(The Binding API gives no guarantees) so, it makes zero sense that the compiler is assessing correctness on the basis of the enclosing function's isolation.

1 Like

Function values default to non-Sendable, so this signature implicitly promises that get and set won't cross an isolation domain. If Binding allows that to happen internally, then either the interface is misrepresented (and these values should be @Sendable) or there's an implementation bug that violates the concurrency rules.

This can be reproduced locally fairly easily. On its own, the following is fine:

func f(_ h: @escaping () -> Void) {}

actor A {
    var x: Int = 0
    func g() {
        f {
            self.x += 1
        }
    }
}

but if the body of f is changed to:

    Task.detached {
        h()
    }

then we get a warning that a non-Sendable closure is being captured by a Sendable closure. If we mark h as @Sendable, then we do get an error at the call site of f:

test.swift:12:18: error: actor-isolated property 'x' can not be mutated from a Sendable closure
            self.x += 1
                 ^
test.swift:9:9: note: mutation of this property is only permitted within the actor
    var x: Int = 0
        ^
1 Like

This is going beyond my understanding.

Are you saying that if I write a library that has a callback (just a regular escaping function) - then somehow the library code needs to know what isolation domain the callback was created in - and ensure to only call it in that domain?

That seems kinda weird if so. How would my library even know what isolation domain the callback was created in? I just saved it to a variable, then called it at a later point...

Well, yes and no. It doesn't need to know which specific isolation domain the callback was created in, but if it's a non-sendable function then the callee does know that the function was created in the current isolation domain (because non-sendable functions can't cross isolation domains). So what the callee is responsible for ensuring is that the function doesn't leave the current isolation domain, which should be covered by sendability checking. Moving a non-sendable function (or any non-sendable value) to another isolation domain is invalid.

The problem here is not with the Binding – which isn’t Sendable and should not be – but with Task.detached { val = false }, which improperly captures the Binding in a non-@Sendable closure, and this is correctly diagnosed with full strict concurrency checking. I personally think this should be an error rather than a warning, but :man_shrugging:t2:

In the end I traced this crash to something related to the Secure Enclave ECC encryption that I’m working with, but then it was inadvertently resolved by another fix that I implemented, and that other fix may well have had an effect on the interweaving of background and @MainActor methods. I don’t know to what extent Secure Enclave encryption may involve the Objective-C-related things that expose one to these @MainActor foot-guns, but I just wanted to post this here in case the information I’ve provided sparks any helpful feedback in response, because inadvertently fixing a completely cryptic crash that reproduces 100% of the time on my boss’ TestFlight instance and 0% of the time on my devices is unsettling…