GlobalActor directive doesn't guarantee a function will be called on that actor

Question

Assuming I have defined a global actor:

@globalActor actor MyActor {
    static let shared = MyActor()
}

And I have a class, in which a couple of methods need to act under this:

class MyClass {

    @MyActor func doSomething(undoManager: UndoManager) {

        // Do something here
   
        undoManager?.registerUndo(withTarget: self) { 
            $0.reverseSomething(undoManager: UndoManager)
        }
    }

    @MyActor func reverseSomething(undoManager: UndoManager) {

        // Do the reverse of something here

        print(\(Thread.isMainThread) /// Prints true when called from undo stack
   
        undoManager?.registerUndo(withTarget: self) { 
            $0.doSomething(undoManager: UndoManager)
        }
    }
}

Assume the code gets called from a SwiftUI view:

struct MyView: View {
   @Environment(\.undoManager) private var undoManager: UndoManager?
   let myObject: MyClass

   var body: some View {
        Button("Do something") { myObject.doSomething(undoManager: undoManager) }
   }
}

Note that when the action is undone the 'reversing' func it is called on the MainThread. Is the correct way to prevent this to wrap the undo action in a task? As in:

    @MyActor func reverseSomething(undoManager: UndoManager) {

        // Do the reverse of something here

        print(\(Thread.isMainThread) /// Prints true
   
        undoManager?.registerUndo(withTarget: self) { 
            Task { $0.doSomething(undoManager: UndoManager) }
        }
    }

Rob · Answer 1 · 2022-10-13T06:01:59.753

I am surprised that the compiler does not generate a warning about calling a global actor 'MyActor'-isolated instance method in a synchronous nonisolated context (i.e. the closure). It would appear that the compiler is confused by the closure syntax within an actor isolated method.

Anyway, you can wrap it in a Task and it should run that on the appropriate actor:

@MyActor func doSomething(undoManager: UndoManager) {
    // Do something here

    undoManager.registerUndo(withTarget: self) { target in
        Task { @MyActor in
            target.reverseSomething(undoManager: undoManager)
        }
    }
}

That having been said, I have found erratic UndoManager behavior when using it from a background thread (i.e., not on the main actor).

So, especially because undo/redo is behavior generally initiated from the UI (on the main thread), I would keep it on the main thread, and only run the desired work on another actor. E.g.:

struct ContentView: View {
    @StateObject var viewModel = ViewModel()
    @State var input: String = ""

    var body: some View {
        VStack {
            TextField(text: $input) {
                Text("enter value")
            }

            Button("Add record") {
                viewModel.addAndPrepareUndo(for: input)
                input = ""
            }.disabled(input.isEmpty)

            Button("Undo") {
                viewModel.undo()
            }.disabled(!viewModel.canUndo)

            Button("Redo") {
                viewModel.redo()
            }.disabled(!viewModel.canRedo)
        }
        .padding()
    }
}

@globalActor actor MyGlobalActor {
    static let shared = MyGlobalActor()
}

@MainActor
class ViewModel: ObservableObject {
    @MyGlobalActor
    var values: [String] = []

    @Published var canUndo = false
    @Published var canRedo = false

    private var undoManager = UndoManager()

    func undo() {
        undoManager.undo()
        updateUndoStatus()
    }

    func redo() {
        undoManager.redo()
        updateUndoStatus()
    }

    func updateUndoStatus() {
        canUndo = undoManager.canUndo
        canRedo = undoManager.canRedo
    }

    func addAndPrepareUndo(for newValue: String) {
        undoManager.registerUndo(withTarget: self) { [weak self] target in
            guard let self else { return }
            self.removeAndPrepareRedo(for: newValue)
        }
        updateUndoStatus()

        Task { @MyGlobalActor in
            values.append(newValue)
            print(#function, values)
        }
    }

    func removeAndPrepareRedo(for revertValue: String) {
        undoManager.registerUndo(withTarget: self) { [weak self] target in
            guard let self else { return }
            self.addAndPrepareUndo(for: revertValue)
        }
        updateUndoStatus()

        Task { @MyGlobalActor in
            values.removeLast()
            print(#function, values)
        }
    }
}

Now, this is a somewhat contrived example (for something this simple, we wouldn't have a simply array on a global actor), but hopefully it illustrates the idea.

Or, you can use a non-global actor:

struct ContentView: View {
    @StateObject var viewModel = ViewModel()
    @State var input: String = ""

    var body: some View {
        VStack {
            TextField(text: $input) {
                Text("enter value")
            }

            Button("Add record") {
                viewModel.addAndPrepareUndo(for: input)
                input = ""
            }.disabled(input.isEmpty)

            Button("Undo") {
                viewModel.undo()
            }.disabled(!viewModel.canUndo)

            Button("Redo") {
                viewModel.redo()
            }.disabled(!viewModel.canRedo)
        }
        .padding()
    }
}

@MainActor
class ViewModel: ObservableObject {
    var model = Model()

    @Published var canUndo = false
    @Published var canRedo = false

    private var undoManager = UndoManager()

    func undo() {
        undoManager.undo()
        updateUndoStatus()
    }

    func redo() {
        undoManager.redo()
        updateUndoStatus()
    }

    func updateUndoStatus() {
        canUndo = undoManager.canUndo
        canRedo = undoManager.canRedo
    }

    func addAndPrepareUndo(for newValue: String) {
        undoManager.registerUndo(withTarget: self) { [weak self] target in
            guard let self else { return }
            self.removeAndPrepareRedo(for: newValue)
        }
        updateUndoStatus()

        Task {
            await model.append(newValue)
            await print(#function, model.values())
        }
    }

    func removeAndPrepareRedo(for revertValue: String) {
        undoManager.registerUndo(withTarget: self) { [weak self] target in
            guard let self else { return }
            self.addAndPrepareUndo(for: revertValue)
        }
        updateUndoStatus()

        Task {
            await model.removeLast()
            await print(#function, model.values())
        }
    }
}

actor Model {
    private var strings: [String] = []

    func append(_ string: String) {
        strings.append(string)
    }

    func removeLast() {
        strings.removeLast()
    }

    func values() -> [String] {
        strings
    }
}

I'm using Xcode 13.4 so perhaps this is resolved in 14. If not I'll raise a feedback request. Thanks for sharing the `Task { @MyActor in ...` syntax. — Philip Pegden, Jul 11 '22 at 13:42
Your code doesn't compile. Even after fixing all the syntax error's you get the error in the button's action: 'Call to global actor 'MyActor'-isolated instance method 'doSomething(undoManager:)' in a synchronous main actor-isolated context' — fruitcoder, Oct 12 '22 at 19:48

GlobalActor directive doesn't guarantee a function will be called on that actor

1 Answers1