Unable to cancel effects when the view is dismissed at the exact moment

[dazy1030]

Description

If you close a screen that was pushed with navigationDestination at just the right moment, the next effect continues to run. The code below demonstrates the issue. You can reproduce it by tapping the navigation bar’s back button or calling dismiss while the fetch1 action is still being processed.

enum Action {
  case onAppear
  case fetchResult1
  case fetchResult2
}

var body: some ReducerOf<Self> {
  Reduce { state, action in
    switch action {
    case .onAppear:
      return .run { send in
        @Dependency(\.continuousClock) var clock
        try await clock.sleep(for: .seconds(1))
        await send(.fetchResult1)
      }
    case .fetchResult1:
      // press back button
      // or
      // Task {
      //   @Dependency(\.dismiss) var dismiss
      //   await dismiss()
      // }
      return .run { send in
        @Dependency(\.continuousClock) var clock
        try await clock.sleep(for: .seconds(1))
        await send(.fetchResult2)
      }
    case .fetchResult2:
      return .none
    }
  }
}

Checklist

• I have determined whether this bug is also reproducible in a vanilla SwiftUI project.
• If possible, I've reproduced the issue using the main branch of this package.
• This issue hasn't been addressed in an existing GitHub issue or discussion.

Expected behavior

Do not run the effect emitted by fetchResult1, or cancel it immediately after it starts.

Actual behavior

The effect is processed and fetchResult2 gets dispatched, which triggers a runtime warning from PresentationReducer’s reduce method.

Reproducing project

https://github.com/dazy1030/AppPlayground/tree/tca/dismiss-cancel

The Composable Architecture version information

1.18.0, 1.19.1, main(abd1331)

Destination operating system

iOS 18.1

Xcode version information

Version 16.2 (16C5032a)

Swift Compiler version information

swift-driver version: 1.115.1 Apple Swift version 6.0.3 (swiftlang-6.0.3.1.10 clang-1600.0.30.1)
Target: arm64-apple-macosx15.0

[lukeredpath]

I’ve been seeing evidence of this problem in our app too, where an effect is kicked off from the action sent from another effect, the user hits the back button and the second effect does not get cancelled, causing a runtime warning when the effect sends an action back to the store.

[dazy1030]

I have confirmed that this issue also occurs on the root-store-isolated-cancellation branch. I tested two methods for closing the screen:

• calling dismiss()
• tapping the back button while blocking the main thread with a sleep function.

case .fetchResult1:
    Task {
        @Dependency(\.dismiss)
        await dismiss()
    }
    return .run { send in
      ...
    }

case .fetchResult1:
    sleep(3)
    // press back button
    return .run { send in
      ...
    }

[lukeredpath]

@dazy1030 I can't completely reproduce this using your project. The Task { } that calls dismiss in the reducer for fetchResult1 does trigger the purple warning although I'm not sure if calling dismiss from a Task is correct anyway. If I hit the back button while it is sleeping in the effect returned from fetchResult1 I don't see any purple runtime warning.

I do also reproduce the issue if you hit the back button while the reducer is sleeping; as this blocks the main thread the back button tap doesn't actually register until the sleep completes but I do get the purple runtime warning.

[lukeredpath]

I've been trying to do some print debugging and whilst I haven't completely got to the bottom of this yet, I am observing that when the dismiss effect is triggered, it is cancelling the presented effects before the effect returned from fetchResult1 has actually started, which means it never sees the cancellation.

[lukeredpath]

I have another hunch about what is happening here. I think I have actually encountered this issue before in a different context. I have a high-order "loadable" reducer that we use throughout our app and in the implementation I just found this comment:

// There is a race condition bug with TCA cancellation where calling .cancellable on
        // an async `.run` effect causes the cancellable to be set up inside the async task
        // - this means it is possible for another action to be received by the store that
        // attempts to cancel this effect before this effect starts executing - because the
        // .cancel effect runs synchronously it can end up happening too soon. By wrapping
        // the `.run` inside a `.merge` with a publisher effect, and calling `.cancellable`
        // on _that_, the publisher implementation of cancellable is used instead and this
        // sets up the cancellable synchronously.

I wrote the above comment and I can't remember how I came across this problem before, but I was seeing that effects were running when I expected them to be cancelled. The workaround was to merge the run effect with a publisher effect - the resulting merged effect would become a publisher effect and calling cancellable on that would set up the cancellation synchronously:

https://github.com/Shimmur/swift-composable-loadable-state/blob/dd294bef1eab255a0932e74b5fd72ce99f111668/Sources/Loadable/LoadableReducer.swift#L118-L140

Could this be the issue with cancellation of presented child/stack effects?

[lukeredpath]

@dazy1030 so I think based on the above I may have found a fix for this, which is applying the same fix I made above directly to Effect.cancellable which is to have it always defer to the Combine implementation and not use await withTaskCancellation because this sets up cancellation asynchronously, whereas the Combine implementation sets up the cancellable synchronously, as soon as the effect is returned.

This means if you try and deliberately introduce a race condition, such as calling dismiss() from an unstructured Task { } the effect cancellable will always win the race and get cancelled as expected.

Can you give this fork a try and see how it works for you?
main...lukeredpath:swift-composable-architecture:lukeredpath/cancellation-race-condition-fix

The downside here is that I guess the long term plan is to remove the Combine dependency so it would be good if there was a better solution to this.

Unfortunately this seems to break the grandchild cancellation test and I can't figure out why - with a bit of print debugging it looks like the only difference is that the grandchild cancellable gets inserted before the child cancellable:

With task cancellation:

[TCA] Setting up task cancellable with id NavigationID(kind: ComposableArchitecture.NavigationID.Kind.keyPath(\State.grandChild), identifier: nil, tag: nil)
[TCA] Setting up task cancellable with id NavigationID(kind: ComposableArchitecture.NavigationID.Kind.keyPath(\State.child), identifier: nil, tag: nil)
[TCA] Inserting cancellable with id NavigationID(kind: ComposableArchitecture.NavigationID.Kind.keyPath(\State.child), identifier: nil, tag: nil) at path NavigationIDPath(path: [])
[TCA] Inserting cancellable with id NavigationID(kind: ComposableArchitecture.NavigationID.Kind.keyPath(\State.grandChild), identifier: nil, tag: nil) at path NavigationIDPath(path: [ComposableArchitecture.NavigationID(kind: ComposableArchitecture.NavigationID.Kind.keyPath(\State.child), identifier: nil, tag: nil)])
[TCA] cancelling effect with id: NavigationID(kind: ComposableArchitecture.NavigationID.Kind.keyPath(\State.child), identifier: nil, tag: nil) at path NavigationIDPath(path: [])
[TCA] Removing cancellable from storage with cancel id _CancelID(discriminator: ObjectIdentifier(0x000000010af61920), id: AnyHashable(ComposableArchitecture.NavigationID(kind: ComposableArchitecture.NavigationID.Kind.keyPath(\State.grandChild), identifier: nil, tag: nil)), navigationIDPath: ComposableArchitecture.NavigationIDPath(path: [ComposableArchitecture.NavigationID(kind: ComposableArchitecture.NavigationID.Kind.keyPath(\State.child), identifier: nil, tag: nil)]), testIdentifier: nil)
[TCA] Cancellable storage with id _CancelID(discriminator: ObjectIdentifier(0x000000010af61920), id: AnyHashable(ComposableArchitecture.NavigationID(kind: ComposableArchitecture.NavigationID.Kind.keyPath(\State.grandChild), identifier: nil, tag: nil)), navigationIDPath: ComposableArchitecture.NavigationIDPath(path: [ComposableArchitecture.NavigationID(kind: ComposableArchitecture.NavigationID.Kind.keyPath(\State.child), identifier: nil, tag: nil)]), testIdentifier: nil) is empty
[TCA] Removing cancellable from storage with cancel id _CancelID(discriminator: ObjectIdentifier(0x000000010af61920), id: AnyHashable(ComposableArchitecture.NavigationID(kind: ComposableArchitecture.NavigationID.Kind.keyPath(\State.grandChild), identifier: nil, tag: nil)), navigationIDPath: ComposableArchitecture.NavigationIDPath(path: []), testIdentifier: nil)
[TCA] Cancellable storage with id _CancelID(discriminator: ObjectIdentifier(0x000000010af61920), id: AnyHashable(ComposableArchitecture.NavigationID(kind: ComposableArchitecture.NavigationID.Kind.keyPath(\State.grandChild), identifier: nil, tag: nil)), navigationIDPath: ComposableArchitecture.NavigationIDPath(path: []), testIdentifier: nil) is empty
[TCA] Removing cancellable from storage with cancel id _CancelID(discriminator: ObjectIdentifier(0x000000010af61920), id: AnyHashable(ComposableArchitecture.NavigationID(kind: ComposableArchitecture.NavigationID.Kind.keyPath(\State.child), identifier: nil, tag: nil)), navigationIDPath: ComposableArchitecture.NavigationIDPath(path: []), testIdentifier: nil)

With publish cancellation:

[TCA] Setting up publisher cancellable with id NavigationID(kind: ComposableArchitecture.NavigationID.Kind.keyPath(\State.grandChild), identifier: nil, tag: nil)
[TCA] Setting up publisher cancellable with id NavigationID(kind: ComposableArchitecture.NavigationID.Kind.keyPath(\State.child), identifier: nil, tag: nil)
[TCA] Inserting cancellable with id NavigationID(kind: ComposableArchitecture.NavigationID.Kind.keyPath(\State.grandChild), identifier: nil, tag: nil) at path NavigationIDPath(path: [ComposableArchitecture.NavigationID(kind: ComposableArchitecture.NavigationID.Kind.keyPath(\State.child), identifier: nil, tag: nil)])
[TCA] Inserting cancellable with id NavigationID(kind: ComposableArchitecture.NavigationID.Kind.keyPath(\State.child), identifier: nil, tag: nil) at path NavigationIDPath(path: [])
[TCA] cancelling effect with id: NavigationID(kind: ComposableArchitecture.NavigationID.Kind.keyPath(\State.child), identifier: nil, tag: nil) at path NavigationIDPath(path: [])
[TCA] Removing cancellable from storage with cancel id _CancelID(discriminator: ObjectIdentifier(0x000000010af618d0), id: AnyHashable(ComposableArchitecture.NavigationID(kind: ComposableArchitecture.NavigationID.Kind.keyPath(\State.grandChild), identifier: nil, tag: nil)), navigationIDPath: ComposableArchitecture.NavigationIDPath(path: []), testIdentifier: nil)
[TCA] Cancellable storage with id _CancelID(discriminator: ObjectIdentifier(0x000000010af618d0), id: AnyHashable(ComposableArchitecture.NavigationID(kind: ComposableArchitecture.NavigationID.Kind.keyPath(\State.grandChild), identifier: nil, tag: nil)), navigationIDPath: ComposableArchitecture.NavigationIDPath(path: []), testIdentifier: nil) is empty
[TCA] Removing cancellable from storage with cancel id _CancelID(discriminator: ObjectIdentifier(0x000000010af618d0), id: AnyHashable(ComposableArchitecture.NavigationID(kind: ComposableArchitecture.NavigationID.Kind.keyPath(\State.child), identifier: nil, tag: nil)), navigationIDPath: ComposableArchitecture.NavigationIDPath(path: []), testIdentifier: nil)
[TCA] Cancellable storage with id _CancelID(discriminator: ObjectIdentifier(0x000000010af618d0), id: AnyHashable(ComposableArchitecture.NavigationID(kind: ComposableArchitecture.NavigationID.Kind.keyPath(\State.child), identifier: nil, tag: nil)), navigationIDPath: ComposableArchitecture.NavigationIDPath(path: []), testIdentifier: nil) is empty

[lukeredpath]

@stephencelis we've spoke a few times in Slack about the issues I've been seeing in our production app with this cancellation thread race issue - do you have any ideas on how this can be avoided? Its very difficult to reproduce locally because it is a thread race, which means in practice it happens very intermittently and requires the volume of actions for it to show up (which is why we only seem to see it in our release builds).

As above, I'm pretty sure the race is due to the asynchronous nature of cancellable .run effects versus actions that trigger cancellation - even if the cancellation effect (e.g. due to a dismiss or pop) starts after a child effect, the cancellation happens before the withTaskCancellation call which means the child effect never sees the cancellation.

I'm not sure if my above attempt to fix this is a viable long-term solution as it relies on the synchronous nature of publisher effect cancellation and I also couldn't resolve a single failing test on that branch.

I wonder if there is possibly something that could be done with task priorities, to ensure child effects have a higher priority than cancellation effects. That might not be entirely deterministic but it might help.