Using State in effect calculation can lead to crashes with @ObservableState

[KaiOelfke]

In one app I used the same struct (for convenience of writing less code) for a reducer state and expensive calculations off the main thread. Once the effect is done it's sending an action with the results, which I feed back into the current state. As the calculation is based on state I also added cancellation logic to cancel in flight effects and redo calculations, if the calculation input data changed.

With ObservableState this doesn't work anymore. Making a copy of the state struct was previously fine. But with ObservableState it crashed my app as it triggered the observation on change off the main thread.

I guess it' was never the best pattern to use the same struct, but back then it was the quickest way to implement it as my struct was quite big and needing those values to calculate statistics.

The problem can be fixed by using a different struct or making a new struct instance copying only values, but not the observation ID so no on change is triggered as the calculation struct isn't observed.

But I'm wondering if the TCA observation logic should runtime warn against being on a non main thread when triggering on change.

Minimal sample. Sometimes you need to run it more times to get into the assertion.

@Reducer
struct Feature {
    @ObservableState
    struct State {
        var int = 42
    }
    enum Action {
        case onAppear
        case receiveUpdate(State)
    }
    var body: some ReducerOf<Self> {
        Reduce { state, action in
            switch action {
            case .onAppear:
                // Expensive calculation in effect
                return .run { [state] send in
                    // Copying constant value type was previously safe to have all the state to do the calculation
                    // With @ObservableState it's not safe as the change will notify observers off the main thread
                    // and potentially crash.
                    var copy = state
                    var iter = 0
                    while iter < 10 {
                        // Update state with some calculation result
                        copy.int = Int.random(in: 0...10)
                        try await Task.sleep(nanoseconds: NSEC_PER_MSEC)
                        iter += 1
                    }
                    // Send results
                    await send(.receiveUpdate(copy))
                }
            case .receiveUpdate(let update):
                state.int = update.int
                return .none
            }
        }
    }
}

struct ContentView: View {
    let store: StoreOf<Feature> = .init(initialState: .init()) {
        Feature()
    }
    var body: some View {
        WithPerceptionTracking {
            VStack {
                let _ = assert(Thread.isMainThread)
                Text(store.int.description)
            }
            .padding()
            .onAppear {
                store.send(.onAppear)
            }
        }
    }
}

[jshier]

Perhaps something like a nonobservable property could reexpose the original state to allow whole value mutations like this? State could then take the value as input.

[stephencelis]

I think this behavior is understandable, and it is technically reproducible in vanilla SwiftUI, but because classes aren't trivially sendable it typically comes with warnings.

For now it can be addressed in an app by:

1. Avoiding capturing the entirety of state in an effect. Instead pluck out the fields that are needed.
2. Mark the effect @MainActor. Even if an expensive computation is performed in an effect, the local effect's context can usually be @MainActor and not tie things up.

We're open to other ideas, though. If anyone thinks that we can leverage concurrency and its diagnostics better, please let us know!

[KaiOelfke]

It took me 15-30 min to understand the crash, but I eventually figured from the episodes that the observation machinery with the id was copied and triggering it.

As a Swift programmer I have different expectations when working with value types and reference types. I normally expect copying a struct like this to not have any side effects like triggering observation. So to some degree this behavior is against my expectations even though I understand it now. For classes I don't have these expectations, it'll have all these Sendable issues as you say and if it's used for Observation it'll be most likely MainActor isolated. At least observation in SwiftUI would require that the model changes on the MainActor. And I want to mutate a single value and not shared state. A copied class would be a copied reference to the same instance so this would be mutating shared state while calculating. So I wouldn't use a reference type.

1. For a few fields this a good approach, but it wouldn't be nice in my case as it's > 10 fields
2. If there's no performance issues it would work yes

Is there any valid use case for on observation on change trigger for ObservableState outside of TCA / SwiftUI that is ok to not be on the main thread? Because if there's no use case an runtime warning would help. Non main thread observations onChange triggers due to code from above do not always crash. So they can be unnoticed, get released and then result in many crashes.

I like the idea of @jshier. Maybe ObservableState could provide a nonobservale copy or at least one with a new id. Because I fixed this for now by writing my manual copy function that creates a new struct value with all fields I need except observation stuff.

[mbrandonw]

As a Swift programmer I have different expectations when working with value types and reference types.

That's definitely understandable, but we also all know that there are "value" types out there that secretly have references buried deep inside. And we deal with those kinds of things everyday, and are familiar with their quirks. For example, @State is a "value" type on the surface, but clearly has some very strange behavior in that it can be mutated inside a struct view. Same with Binding, as well as our new @Shared type.

Sometimes in order to get the best of both worlds of values and references, you need to blend the two together and be aware of how they work together. @ObservableState allows us to keep State as a "value" type for the most part, but at the end of the day it imbues State with some reference semantics. And so it's just important to be aware of that.

Maybe ObservableState could provide a nonobservale copy or at least one with a new id. Because I fixed this for now by writing my manual copy function that creates a new struct value with all fields I need except observation stuff.

This will make it possible for you to have very subtle bugs. If you do state.copy() in the effect to get a copy of the state with a new registrar, and then do state = copy later in the reducer, it will clear out the previous registrar with a whole new one. That will mean the access list has been reset, and so it will be possible for state to change without updating the UI.

We could potentially have a tool like this:

withoutObservationTracking {
  state.count += 1
  // Any mutation to `state` in here will not trigger tracking.
}

But then we are going to incur the cost of a TaskLocal on all mutations just for this edge case.

We personally think the strategy of simply capturing the fields you want in the effect is the way to go, even if a little verbose. And maybe someday in the future we can leverage the new ~Copyable and ~Escapable tools in Swift in order to have the compiler catch us earlier when we are doing something with observable state that is not correct.

[KaiOelfke]

These are good points and make sense to me. But I'm still wondering, if a runtime warning for triggering onChange on ObservableState on a non main thread is useful. Or if there's use cases where this is normal.

[mbrandonw]

Hi @KaiOelfke, we may consider adding a runtime warning, but it is not a high priority for us right now.