swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.26k stars 10.33k forks source link

The limitations of @MainActor are not clearly documented #62031

Open ConfusedVorlon opened 1 year ago

ConfusedVorlon commented 1 year ago

Describe the bug

@MainActor is broadly described as an annotation by which the compiler will guarantee that a function is called on the main thread. For example

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.

However this is far from true. There are loads of cases where a function marked as @MainActor will be called on a background thread.

The Swift Programming Language discusses concurrency, but has only a single passing reference to @MainActor

Devs are left to guess what is really going on - or to trust (frequently incorrect) web tutorials

Expected behavior

Swift should provide a clear statement of what the @MainActor annotation does, and what limitations exist

My assumption is that the compiler-based approach to Swift Concurrency means that there will always be multiple ways where the 'guarantee' fails.

This is fine as long as it is clearly documented.

Additional context

Example ways to break Swift Concurrency, taken from my blog post on the subject https://blog.hobbyistsoftware.com/2022/11/five-ways-to-break-swift-concurrency/

All these methods result in noCanFail being called on a background thread - and consequently failing

I'm not suggesting that these are bugs in Swift Concurrency (though perhaps some of them could at least be warnings) - rather that in the absence of documentation, they represent hidden traps.

class OnMain:NSObject {

    @MainActor
    @objc
    func noCanFail() {
        guard Thread.isMainThread else {
            fatalError("not on main")
        }
        print("Did Not Fail!")
    }
    //Just call the selector directly. Swift doesn't care
    func breakWithSelector() {
        Task {
            self.perform(#selector(self.noCanFail))
        }
    }

    // Get some Objective C code to call the 'protected' function
    // The objective C code is simply
    // [[OnMain new] noCanFail];
    func breakByCallingObjC() {
        Task {
            Trouble.cause()
        }
    }

    //NSNotification centre calls you back on whatever thread the notification was posted
    //It doesn't care about Swift Concurrency
    func breakByNotification() {
        let notification = Notification.Name("Cheeky")
        let center = NotificationCenter.default

        center.addObserver(self,
                           selector: #selector(self.noCanFail),
                           name: notification,
                           object: nil)
        Task {
            center.post(name: notification, object: nil)
        }
    }

    //Surprisingly, even with block syntax, you get the fatal error
    //At least there is a warning in this one
    func breakByNotification2() {
        let notification2 = Notification.Name("Cheeky2")

        let queue = OperationQueue()
        let center = NotificationCenter.default

        center.addObserver(forName: notification2,
                           object: nil,
                           queue: queue) { _ in
            self.noCanFail()
        }

        //No need to post from a task this time!
        center.post(name: notification2, object: nil)
    }

    //Objective C will just run the block for us by calling
    //block();
    //At least there is a warning in this one
    func breakWithBlock() {
        Task {
            Trouble.run {
                self.noCanFail()
            }
        }
    }

}

the implementation of Trouble is simply

@implementation Trouble

+(void)cause{
    [[OnMain new] noCanFail];
}

+(void)run:(void (^)(void))block {
    block();
}

@end

similarly, here is an example where inheriting @MainActor from a parent class leads to the same failure mode

class Foo:NSViewController {
}
//perhaps conforming to some delegate protocol...
extension Foo:MyLibraryDelegate {
    func someCallback() {
        OnMain().noCanFail()
    }
}

where the library is simply

protocol MyLibraryDelegate {
    func someCallback()
}
class MyLibrary {
    var delegate:MyLibraryDelegate

    init(delegate:MyLibraryDelegate){
        self.delegate = delegate
    }

    func doWork() {
        Task {
            delegate.someCallback()
        }
    }
}
tbkka commented 1 year ago

CC: @kavon @DougGregor

AnthonyLatsis commented 1 year ago

@ktoso What‘s your take on this? Does a transfer to swift-book make sense to you?