allow processing of sync actions resulting from state update (#1360)

* allow processing of sync actions resulting from state update

Sending an action into the system synchronously from a ViewStore's publisher
can result in state updates publishing the wrong, old, value.

This PR fixes it by keeping the `Store` in `sending` state while
we publish state updates. Once that's done, we check if there
were any new actions coming in, in which case we feed them back and
start processing again.

* Clean up and clarify behavior

Co-authored-by: Stephen Celis <[email protected]>
(cherry picked from commit 0855c9952549173fc7c655fcee31ffbb709e2dac)

# Conflicts:
#	Sources/ComposableArchitecture/Store.swift

Author: diederich

Sources/ComposableArchitecture/Store.swift

@@ -338,12 +338,26 @@ public final class Store<State, Action> {
     var currentState = self.state
     let tasks = Box<[Task<Void, Never>]>(wrappedValue: [])
     defer {
+      // NB: Handle any potential re-entrant actions.
+      //
+      // 1. After all buffered actions have been processed, we clear them out, but this can lead to
+      //    re-entrant actions if a cleared action holds onto an object that sends an additional
+      //    action during "deinit".
+      //
+      //    We use "withExtendedLifetime" to prevent simultaneous access exceptions for this case,
+      //    which otherwise would try to append an action while removal is still in-process.
       withExtendedLifetime(self.bufferedActions) {
         self.bufferedActions.removeAll()
       }
-      self.isSending = false
+      // 2. Updating state can also lead to re-entrant actions if the emission of a downstream store
+      //    publisher sends an action back into the store.
+      //
+      //    We update "state" _before_ flipping "isSending" to false to ensure these actions are
+      //    appended to the buffer and not processed immediately.
       self.state = currentState
-      // NB: Handle any re-entrant actions
+      self.isSending = false
+      // Should either of the above steps send re-entrant actions back into the store, we handle
+      // them recursively.
       if !self.bufferedActions.isEmpty {
         if let task = self.send(
           self.bufferedActions.removeLast(), originatingFrom: originatingAction
@@ -374,24 +388,24 @@ public final class Store<State, Action> {
             }
           },
           completed: { [weak self] in
-            self?.threadCheck(status: .effectCompletion(action))
-            boxedTask.wrappedValue?.cancel()
-            didComplete = true
+              self?.threadCheck(status: .effectCompletion(action))
+              boxedTask.wrappedValue?.cancel()
+              didComplete = true
             self?.effectDisposables.removeValue(forKey: uuid)?.dispose()
-          },
+            },
           interrupted: { [weak self] in
             boxedTask.wrappedValue?.cancel()
             didComplete = true
             self?.effectDisposables.removeValue(forKey: uuid)?.dispose()
-          }
+              }
         )
 
         let effectDisposable = CompositeDisposable()
         effectDisposable += producer.start(observer)
         effectDisposable += AnyDisposable { [weak self] in
           self?.threadCheck(status: .effectCompletion(action))
           self?.effectDisposables.removeValue(forKey: uuid)?.dispose()
-        }
+            }
 
         if !didComplete {
           let task = Task<Void, Never> { @MainActor in

Tests/ComposableArchitectureTests/CompatibilityTests.swift

@@ -4,7 +4,7 @@ import XCTest
 
 @MainActor
 final class CompatibilityTests: XCTestCase {
-  func testCaseStudy_ReentrantEffect() {
+  func testCaseStudy_ReentrantActionsFromBuffer() {
     let cancelID = UUID()
 
     struct State: Equatable {}
@@ -72,6 +72,53 @@ final class CompatibilityTests: XCTestCase {
       ]
     )
   }
+
+  func testCaseStudy_ReentrantActionsFromProducer() {
+    struct State: Equatable {
+      var city: String
+      var country: String
+    }
+
+    enum Action: Equatable {
+      case updateCity(String)
+      case updateCountry(String)
+    }
+
+    let reducer = Reducer<State, Action, Void> { state, action, _ in
+      switch action {
+      case let .updateCity(city):
+        state.city = city
+        return .none
+      case let .updateCountry(country):
+        state.country = country
+        return .none
+      }
+    }
+
+    let store = Store(
+      initialState: State(city: "New York", country: "USA"),
+      reducer: reducer,
+      environment: ()
+    )
+    let viewStore = ViewStore(store)
+
+    viewStore.produced.city
+      .startWithValues { city in
+        if city == "London" {
+          viewStore.send(.updateCountry("UK"))
+        }
+      }
+
+    var countryUpdates = [String]()
+    viewStore.produced.country
+      .startWithValues { country in
+        countryUpdates.append(country)
+      }
+
+    viewStore.send(.updateCity("London"))
+
+    XCTAssertEqual(countryUpdates, ["USA", "UK"])
+  }
 }
 
 private final class OnDeinit: Equatable {