Search code examples
swiftasync-awaitconcurrencyactor

Mutating a mutable Struct within a Swift Task


We are experiencing a few crashes on a diffable data source.

The view model that holds the business logic has few structs marked as var, hence they are mutable.

Those structs can be changed on the main thread or in an event handler that wraps a Swift Task.

I'm wondering if within the Task is safe to mutating structs (adding or removing elements) even if the structs are sendable by default. On the contrary, I'm asking if they should be accessed by means of an Actor.

final class MyModelRepository {
    private var items: [MyModel] = []
    private(set) var uniqueKeys = Set<String>()

    // run on the main thread
    func removeAll() {
        items.removeAll()
        uniqueKeys.removeAll()
    }

    ...

    // scheduled through a scheduledTimer    
    func eventHandler() {
        Task {
            do {
                let updates = try await retrieveUpdates() // call an API service to retrieve the updated updates
                updateItemsPublisher.send(updates.updatedValues) // sinked on main thread
                for item in updates.removed {
                    items.removeAll { $0 == item }
                    uniqueKeys.remove(item.key.orEmpty)
                    guard let deletedModel = // find the deleted model locally to the app
                    deleteItemPusblisher.send(deletedModel) // sinked on main thread
                }
            } catch {
                // log the error here
            }
        }
    }
}

Solution

  • You asked:

    I'm wondering if within the Task is safe to [mutate] structs.

    As a general question, it is safe to do so if and only if:

    • the property is actor-isolated, and

    • you are invoking Task {…} from a method that is isolated to the same actor.

    Task {…} creates a new top-level task on behalf of the current actor (if any). So, no, you don’t have to introduce a new actor if it is already actor-isolated.

    Now, in your example, there are two problems.

    1. We are not in an actor-isolated context. We would generally make the type an actor rather than a class, or we would isolate this class to some global actor (such as @MainActor). See

    2. Even if we were, you suggest that eventHandler is being called by a Timer, i.e., not from a Swift concurrency context. In Swift concurrency, rather than a Timer we might have a task with a try await Task.sleep call, which would preserve whatever actor-isolated context from which you invoked it.

    I might suggest you see WWDC 2021’s Protect mutable state with Swift actors or 2022’s Eliminate data races using Swift Concurrency


    Here are some safe and unsafe examples:

    // struct
    struct Foo {
        var bar: String
    }
    
    // unsafe because neither property nor method are actor-isolated; `Task {…}` is not actor isolated, either
    class UnsafeClassExample {
        var foo = Foo(bar: "baz")
        
        func thisIsUnsafe() {
            Task {
                foo.bar = "qux"
            }
        }
    }
    
    // safe because actor (thus property and method are actor-isolated); `Task {…}` will be actor isolated, too
    actor SafeActorExample {
        var foo = Foo(bar: "baz")
        
        func thisIsSafe() {
            Task {
                foo.bar = "qux"
            }
        }
    }
    
    // safe because whole class (including both property and method) is actor-isolated; `Task {…}` will be actor isolated, too
    @MainActor
    class SafeActorIsolatedClassExample {
        var foo = Foo(bar: "baz")
        
        func thisIsSafe() {
            Task {
                foo.bar = "qux"
            }
        }
    }
    
    // safe because both property and method are actor-isolated to same global actor; `Task {…}` will be actor isolated, too
    class SafeActorIsolatedPropertyAndMethodExample {
        @MainActor var foo = Foo(bar: "baz")
        
        @MainActor func thisIsSafe() {
            Task {
                foo.bar = "qux"
            }
        }
    }
    
    // safe because `UIViewController` is actor-isolated to `@MainActor`, and therefore behaves like `SafeActorIsolatedClassExample`
    class SafeViewController: UIViewController {
        var foo = Foo(bar: "baz")
        
        func thisIsSafe() {
            Task {
                foo.bar = "qux"
            }
        }
    }
    

    Your example, even before we get to the problem that Timer introduces, is not safe, akin to the UnsafeClassExample, above.


    As an aside, I would avoid introducing unstructured concurrency with Task {…}, but the original question was re Task {…}, hence my usage above.