Perform thread check only when store is created on main queue. (#811)

* Perform thread check only when store is created on main thread.

* clean up

* Update Sources/ComposableArchitecture/Store.swift

* clean up

* Update Sources/ComposableArchitecture/Store.swift

* clean up

* execute setSpecific only once.

* logic fix

* added a test

* typo

* wip

* docs

* fix test

* Update Sources/ComposableArchitecture/Store.swift

Co-authored-by: Thomas Grapperon <[email protected]>

* Run swift-format

* Clean up speech recognition case study. (#812)

* Clean up speech recognition case study.

* fix tests

* clean up;

* Alternative to `CurrentValueSubject` in `ViewStore` (#755)

* Replaced CurrentValueSubject with CurrentValueRelay

* Added final to DemandBuffer

Co-authored-by: Brandon Williams <[email protected]>

* Run swift-format

* Fix bindable deprecations (#815)

* Fix Bindable Deprecations

* More CI

* wip

* wip

* wip

* wip

* Run swift-format

* beef up test

* expectation

* fix

Co-authored-by: Stephen Celis <[email protected]>
Co-authored-by: Thomas Grapperon <[email protected]>
Co-authored-by: stephencelis <[email protected]>
Co-authored-by: iampatbrown <[email protected]>
Co-authored-by: mbrandonw <[email protected]>

Author: 5 people

Sources/ComposableArchitecture/Store.swift

@@ -76,9 +76,9 @@ import ReactiveSwift
 /// ### Thread safety
 ///
 /// The `Store` class is not thread-safe, and so all interactions with an instance of ``Store``
-/// (including all of its scopes and derived ``ViewStore``s) must be done on the same thread.
-/// Further, if the store is powering a SwiftUI or UIKit view, as is customary, then all
-/// interactions must be done on the _main_ thread.
+/// (including all of its scopes and derived ``ViewStore``s) must be done on the same thread the
+/// store was created on. Further, if the store is powering a SwiftUI or UIKit view, as is
+/// customary, then all interactions must be done on the _main_ thread.
 ///
 /// The reason stores are not thread-safe is due to the fact that when an action is sent to a store,
 /// a reducer is run on the current state, and this process cannot be done from multiple threads.
@@ -111,6 +111,21 @@ import ReactiveSwift
 /// However, by leaving scheduling out of the ``Store`` we get to test these aspects of our effects
 /// if we so desire, or we can ignore if we prefer. We have that flexibility.
 ///
+/// #### Thread safety checks
+///
+/// The store performs some basic thread safety checks in order to help catch mistakes. Stores
+/// constructed via the initializer ``Store/init(initialState:reducer:environment:)`` are assumed
+/// to run only on the main thread, and so a check is executed immediately to make sure that is the
+/// case. Further, all actions sent to the store and all scopes (see ``Store/scope(state:action:)``)
+/// of the store are also checked to make sure that work is performed on the main thread.
+///
+/// If you need a store that runs on a non-main thread, which should be very rare and you should
+/// have a very good reason to do so, then you can construct a store via the
+/// ``Store/unchecked(initialState:reducer:environment:)`` static method to opt out of all main
+/// thread checks.
+///
+/// ---
+///
 /// See also: ``ViewStore`` to understand how one observes changes to the state in a ``Store`` and
 /// sends user actions.
 public final class Store<State, Action> {
@@ -130,7 +145,7 @@ public final class Store<State, Action> {
   private let reducer: (inout State, Action) -> Effect<Action, Never>
   private var bufferedActions: [Action] = []
   #if DEBUG
-    private let initialThread = Thread.current
+  private let mainQueueChecksEnabled: Bool
   #endif
 
   /// Initializes a store from an initial state, a reducer, and an environment.
@@ -139,13 +154,38 @@ public final class Store<State, Action> {
   ///   - initialState: The state to start the application in.
   ///   - reducer: The reducer that powers the business logic of the application.
   ///   - environment: The environment of dependencies for the application.
-  public init<Environment>(
+  public convenience init<Environment>(
     initialState: State,
     reducer: Reducer<State, Action, Environment>,
     environment: Environment
   ) {
-    self.state = initialState
-    self.reducer = { state, action in reducer.run(&state, action, environment) }
+    self.init(
+      initialState: initialState,
+      reducer: reducer,
+      environment: environment,
+      mainQueueChecksEnabled: true
+    )
+    self.threadCheck(status: .`init`)
+  }
+
+  /// Initializes a store from an initial state, a reducer, and an environment, and the main thread
+  /// check is disabled for all interactions with this store.
+  ///
+  /// - Parameters:
+  ///   - initialState: The state to start the application in.
+  ///   - reducer: The reducer that powers the business logic of the application.
+  ///   - environment: The environment of dependencies for the application.
+  public static func unchecked<Environment>(
+    initialState: State,
+    reducer: Reducer<State, Action, Environment>,
+    environment: Environment
+  ) -> Self {
+    Self(
+      initialState: initialState,
+      reducer: reducer,
+      environment: environment,
+      mainQueueChecksEnabled: false
+    )
   }
 
   /// Scopes the store to one that exposes local state and actions.
@@ -325,8 +365,8 @@ public final class Store<State, Action> {
     self.scope(state: toLocalState, action: { $0 })
   }
 
-  func send(_ action: Action, isFromViewStore: Bool = true) {
-    self.threadCheck(status: .send(action, isFromViewStore: isFromViewStore))
+  func send(_ action: Action, originatingFrom originatingAction: Action? = nil) {
+    self.threadCheck(status: .send(action, originatingAction: originatingAction))
 
     self.bufferedActions.append(action)
     guard !self.isSending else { return }
@@ -345,8 +385,8 @@ public final class Store<State, Action> {
       var didComplete = false
       let uuid = UUID()
       let observer = Signal<Action, Never>.Observer(
-        value: { [weak self] action in
-          self?.send(action, isFromViewStore: false)
+        value: { [weak self] effectAction in
+          self?.send(effectAction, originatingFrom: action)
         },
         completed: { [weak self] in
           self?.threadCheck(status: .effectCompletion(action))
@@ -386,63 +426,97 @@ public final class Store<State, Action> {
 
   private enum ThreadCheckStatus {
     case effectCompletion(Action)
+    case `init`
     case scope
-    case send(Action, isFromViewStore: Bool)
+    case send(Action, originatingAction: Action?)
   }
 
   @inline(__always)
   private func threadCheck(status: ThreadCheckStatus) {
     #if DEBUG
-      guard self.initialThread != Thread.current
+      guard self.mainQueueChecksEnabled && !isMainQueue
       else { return }
 
       let message: String
       switch status {
       case let .effectCompletion(action):
         message = """
-          An effect returned from the action "\(debugCaseOutput(action))" completed on the wrong \
+          An effect returned from the action "\(debugCaseOutput(action))" completed on a non-main \
           thread. Make sure to use ".receive(on:)" on any effects that execute on background \
-          threads to receive their output on the same thread the store was created on.
+          threads to receive their output on the main thread, or create this store via \
+          "Store.unchecked" to disable the main thread checker.
+          """
+
+      case .`init`:
+        message = """
+          "Store.init" was called on a non-main thread. Make sure that stores are initialized on \
+          the main thread, or create this store via "Store.unchecked" to disable the main thread \
+          checker.
           """
 
       case .scope:
         message = """
-          "Store.scope" was called on the wrong thread. Make sure that "Store.scope" is always \
-          called on the same thread the store was created on.
+          "Store.scope" was called on a non-main thread. Make sure that "Store.scope" is always \
+          called on the main thread, or create this store via "Store.unchecked" to disable the \
+          main thread checker.
           """
 
-      case let .send(action, isFromViewStore: true):
+      case let .send(action, originatingAction: nil):
         message = """
-          "ViewStore.send(\(debugCaseOutput(action)))" was called on the wrong thread. Make sure \
-          that "ViewStore.send" is always called on the same thread the store was created on.
+          "ViewStore.send(\(debugCaseOutput(action)))" was called on a non-main thread. Make sure \
+          that "ViewStore.send" is always called on the main thread, or create this store via \
+          "Store.unchecked" to disable the main thread checker.
           """
 
-      case let .send(action, isFromViewStore: false):
+      case let .send(action, originatingAction: .some(originatingAction)):
         message = """
-          An effect emitted the action "\(debugCaseOutput(action))" from the wrong thread. Make \
-          sure to use ".receive(on:)" on any effects that execute on background threads to receive \
-          their output on the same thread the store was created on.
+          An effect returned from "\(debugCaseOutput(originatingAction))" emitted the action \
+          "\(debugCaseOutput(action))" on a non-main thread. Make sure to use ".receive(on:)" on \
+          any effects that execute on background threads to receive their output on the main \
+          thread, or create this store via "Store.unchecked" to disable the main thread checker.
           """
       }
-
+    
       breakpoint(
         """
         ---
         Warning:
 
-        The store was interacted with on a thread that is different from the thread the store was \
-        created on:
+        A store created on the main thread was interacted with on a non-main thread:
 
-        \(message)
+          Thread: \(Thread.current)
 
-          Store created on: \(self.initialThread)
-          Action sent on: \(Thread.current)
+        \(message)
 
         The "Store" class is not thread-safe, and so all interactions with an instance of "Store" \
-        (including all of its scopes and derived view stores) must be done on the same thread.
+        (including all of its scopes and derived view stores) must be done on the main thread.
         ---
         """
       )
     #endif
   }
+
+  private init<Environment>(
+    initialState: State,
+    reducer: Reducer<State, Action, Environment>,
+    environment: Environment,
+    mainQueueChecksEnabled: Bool
+  ) {
+    self.state = initialState
+    self.reducer = { state, action in reducer.run(&state, action, environment) }
+
+    #if DEBUG
+    self.mainQueueChecksEnabled = mainQueueChecksEnabled
+    #endif
+  }
+}
+
+private let mainQueueKey = DispatchSpecificKey<UInt8>()
+private let mainQueueValue: UInt8 = 0
+private var isMainQueue: Bool {
+  _ = setSpecific
+  return DispatchQueue.getSpecific(key: mainQueueKey) == mainQueueValue
 }
+private var setSpecific: () = {
+  DispatchQueue.main.setSpecific(key: mainQueueKey, value: mainQueueValue)
+}()

Tests/ComposableArchitectureTests/StoreTests.swift

@@ -468,4 +468,33 @@ final class StoreTests: XCTestCase {
         .child(2),
       ])
   }
+  
+  func testNonMainQueueStore() {
+    var expectations: [XCTestExpectation] = []
+    for i in 1 ... 100 {
+      let expectation = XCTestExpectation(description: "\(i)th iteration is complete")
+      expectations.append(expectation)
+      DispatchQueue.global().async {
+        let viewStore = ViewStore(
+          Store.unchecked(
+            initialState: 0,
+            reducer: Reducer<Int, Void, XCTestExpectation> { state, _, expectation in
+              state += 1
+              if state == 2 {
+                return .fireAndForget { expectation.fulfill() }
+              }
+              return .none
+            },
+            environment: expectation
+          )
+        )
+        viewStore.send(())
+        DispatchQueue.global().asyncAfter(deadline: .now() + 0.1) {
+          viewStore.send(())
+        }
+      }
+    }
+
+    wait(for: expectations, timeout: 1)
+  }
 }