Skip to content

Commit 1c0dfbc

Browse files
mbrandonwstephencelistgrapperoniampatbrown
authored
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]>
1 parent f92c2ae commit 1c0dfbc

File tree

2 files changed

+134
-31
lines changed

2 files changed

+134
-31
lines changed

Sources/ComposableArchitecture/Store.swift

Lines changed: 105 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,9 @@ import Foundation
7676
/// ### Thread safety
7777
///
7878
/// The `Store` class is not thread-safe, and so all interactions with an instance of ``Store``
79-
/// (including all of its scopes and derived ``ViewStore``s) must be done on the same thread.
80-
/// Further, if the store is powering a SwiftUI or UIKit view, as is customary, then all
81-
/// interactions must be done on the _main_ thread.
79+
/// (including all of its scopes and derived ``ViewStore``s) must be done on the same thread the
80+
/// store was created on. Further, if the store is powering a SwiftUI or UIKit view, as is
81+
/// customary, then all interactions must be done on the _main_ thread.
8282
///
8383
/// The reason stores are not thread-safe is due to the fact that when an action is sent to a store,
8484
/// a reducer is run on the current state, and this process cannot be done from multiple threads.
@@ -111,6 +111,21 @@ import Foundation
111111
/// However, by leaving scheduling out of the ``Store`` we get to test these aspects of our effects
112112
/// if we so desire, or we can ignore if we prefer. We have that flexibility.
113113
///
114+
/// #### Thread safety checks
115+
///
116+
/// The store performs some basic thread safety checks in order to help catch mistakes. Stores
117+
/// constructed via the initializer ``Store/init(initialState:reducer:environment:)`` are assumed
118+
/// to run only on the main thread, and so a check is executed immediately to make sure that is the
119+
/// case. Further, all actions sent to the store and all scopes (see ``Store/scope(state:action:)``)
120+
/// of the store are also checked to make sure that work is performed on the main thread.
121+
///
122+
/// If you need a store that runs on a non-main thread, which should be very rare and you should
123+
/// have a very good reason to do so, then you can construct a store via the
124+
/// ``Store/unchecked(initialState:reducer:environment:)`` static method to opt out of all main
125+
/// thread checks.
126+
///
127+
/// ---
128+
///
114129
/// See also: ``ViewStore`` to understand how one observes changes to the state in a ``Store`` and
115130
/// sends user actions.
116131
public final class Store<State, Action> {
@@ -121,7 +136,7 @@ public final class Store<State, Action> {
121136
private let reducer: (inout State, Action) -> Effect<Action, Never>
122137
var state: CurrentValueSubject<State, Never>
123138
#if DEBUG
124-
private let initialThread = Thread.current
139+
private let mainQueueChecksEnabled: Bool
125140
#endif
126141

127142
/// Initializes a store from an initial state, a reducer, and an environment.
@@ -130,13 +145,38 @@ public final class Store<State, Action> {
130145
/// - initialState: The state to start the application in.
131146
/// - reducer: The reducer that powers the business logic of the application.
132147
/// - environment: The environment of dependencies for the application.
133-
public init<Environment>(
148+
public convenience init<Environment>(
134149
initialState: State,
135150
reducer: Reducer<State, Action, Environment>,
136151
environment: Environment
137152
) {
138-
self.state = CurrentValueSubject(initialState)
139-
self.reducer = { state, action in reducer.run(&state, action, environment) }
153+
self.init(
154+
initialState: initialState,
155+
reducer: reducer,
156+
environment: environment,
157+
mainQueueChecksEnabled: true
158+
)
159+
self.threadCheck(status: .`init`)
160+
}
161+
162+
/// Initializes a store from an initial state, a reducer, and an environment, and the main thread
163+
/// check is disabled for all interactions with this store.
164+
///
165+
/// - Parameters:
166+
/// - initialState: The state to start the application in.
167+
/// - reducer: The reducer that powers the business logic of the application.
168+
/// - environment: The environment of dependencies for the application.
169+
public static func unchecked<Environment>(
170+
initialState: State,
171+
reducer: Reducer<State, Action, Environment>,
172+
environment: Environment
173+
) -> Self {
174+
Self(
175+
initialState: initialState,
176+
reducer: reducer,
177+
environment: environment,
178+
mainQueueChecksEnabled: false
179+
)
140180
}
141181

142182
/// Scopes the store to one that exposes local state and actions.
@@ -314,8 +354,8 @@ public final class Store<State, Action> {
314354
self.scope(state: toLocalState, action: { $0 })
315355
}
316356

317-
func send(_ action: Action, isFromViewStore: Bool = true) {
318-
self.threadCheck(status: .send(action, isFromViewStore: isFromViewStore))
357+
func send(_ action: Action, originatingFrom originatingAction: Action? = nil) {
358+
self.threadCheck(status: .send(action, originatingAction: originatingAction))
319359

320360
self.bufferedActions.append(action)
321361
guard !self.isSending else { return }
@@ -339,8 +379,8 @@ public final class Store<State, Action> {
339379
didComplete = true
340380
self?.effectCancellables[uuid] = nil
341381
},
342-
receiveValue: { [weak self] action in
343-
self?.send(action, isFromViewStore: false)
382+
receiveValue: { [weak self] effectAction in
383+
self?.send(effectAction, originatingFrom: action)
344384
}
345385
)
346386

@@ -363,63 +403,97 @@ public final class Store<State, Action> {
363403

364404
private enum ThreadCheckStatus {
365405
case effectCompletion(Action)
406+
case `init`
366407
case scope
367-
case send(Action, isFromViewStore: Bool)
408+
case send(Action, originatingAction: Action?)
368409
}
369410

370411
@inline(__always)
371412
private func threadCheck(status: ThreadCheckStatus) {
372413
#if DEBUG
373-
guard self.initialThread != Thread.current
414+
guard self.mainQueueChecksEnabled && !isMainQueue
374415
else { return }
375416

376417
let message: String
377418
switch status {
378419
case let .effectCompletion(action):
379420
message = """
380-
An effect returned from the action "\(debugCaseOutput(action))" completed on the wrong \
421+
An effect returned from the action "\(debugCaseOutput(action))" completed on a non-main \
381422
thread. Make sure to use ".receive(on:)" on any effects that execute on background \
382-
threads to receive their output on the same thread the store was created on.
423+
threads to receive their output on the main thread, or create this store via \
424+
"Store.unchecked" to disable the main thread checker.
425+
"""
426+
427+
case .`init`:
428+
message = """
429+
"Store.init" was called on a non-main thread. Make sure that stores are initialized on \
430+
the main thread, or create this store via "Store.unchecked" to disable the main thread \
431+
checker.
383432
"""
384433

385434
case .scope:
386435
message = """
387-
"Store.scope" was called on the wrong thread. Make sure that "Store.scope" is always \
388-
called on the same thread the store was created on.
436+
"Store.scope" was called on a non-main thread. Make sure that "Store.scope" is always \
437+
called on the main thread, or create this store via "Store.unchecked" to disable the \
438+
main thread checker.
389439
"""
390440

391-
case let .send(action, isFromViewStore: true):
441+
case let .send(action, originatingAction: nil):
392442
message = """
393-
"ViewStore.send(\(debugCaseOutput(action)))" was called on the wrong thread. Make sure \
394-
that "ViewStore.send" is always called on the same thread the store was created on.
443+
"ViewStore.send(\(debugCaseOutput(action)))" was called on a non-main thread. Make sure \
444+
that "ViewStore.send" is always called on the main thread, or create this store via \
445+
"Store.unchecked" to disable the main thread checker.
395446
"""
396447

397-
case let .send(action, isFromViewStore: false):
448+
case let .send(action, originatingAction: .some(originatingAction)):
398449
message = """
399-
An effect emitted the action "\(debugCaseOutput(action))" from the wrong thread. Make \
400-
sure to use ".receive(on:)" on any effects that execute on background threads to receive \
401-
their output on the same thread the store was created on.
450+
An effect returned from "\(debugCaseOutput(originatingAction))" emitted the action \
451+
"\(debugCaseOutput(action))" on a non-main thread. Make sure to use ".receive(on:)" on \
452+
any effects that execute on background threads to receive their output on the main \
453+
thread, or create this store via "Store.unchecked" to disable the main thread checker.
402454
"""
403455
}
404-
456+
405457
breakpoint(
406458
"""
407459
---
408460
Warning:
409461
410-
The store was interacted with on a thread that is different from the thread the store was \
411-
created on:
462+
A store created on the main thread was interacted with on a non-main thread:
412463
413-
\(message)
464+
Thread: \(Thread.current)
414465
415-
Store created on: \(self.initialThread)
416-
Action sent on: \(Thread.current)
466+
\(message)
417467
418468
The "Store" class is not thread-safe, and so all interactions with an instance of "Store" \
419-
(including all of its scopes and derived view stores) must be done on the same thread.
469+
(including all of its scopes and derived view stores) must be done on the main thread.
420470
---
421471
"""
422472
)
423473
#endif
424474
}
475+
476+
private init<Environment>(
477+
initialState: State,
478+
reducer: Reducer<State, Action, Environment>,
479+
environment: Environment,
480+
mainQueueChecksEnabled: Bool
481+
) {
482+
self.state = CurrentValueSubject(initialState)
483+
self.reducer = { state, action in reducer.run(&state, action, environment) }
484+
485+
#if DEBUG
486+
self.mainQueueChecksEnabled = mainQueueChecksEnabled
487+
#endif
488+
}
489+
}
490+
491+
private let mainQueueKey = DispatchSpecificKey<UInt8>()
492+
private let mainQueueValue: UInt8 = 0
493+
private var isMainQueue: Bool {
494+
_ = setSpecific
495+
return DispatchQueue.getSpecific(key: mainQueueKey) == mainQueueValue
425496
}
497+
private var setSpecific: () = {
498+
DispatchQueue.main.setSpecific(key: mainQueueKey, value: mainQueueValue)
499+
}()

Tests/ComposableArchitectureTests/StoreTests.swift

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,4 +447,33 @@ final class StoreTests: XCTestCase {
447447
.child(2),
448448
])
449449
}
450+
451+
func testNonMainQueueStore() {
452+
var expectations: [XCTestExpectation] = []
453+
for i in 1 ... 100 {
454+
let expectation = XCTestExpectation(description: "\(i)th iteration is complete")
455+
expectations.append(expectation)
456+
DispatchQueue.global().async {
457+
let viewStore = ViewStore(
458+
Store.unchecked(
459+
initialState: 0,
460+
reducer: Reducer<Int, Void, XCTestExpectation> { state, _, expectation in
461+
state += 1
462+
if state == 2 {
463+
return .fireAndForget { expectation.fulfill() }
464+
}
465+
return .none
466+
},
467+
environment: expectation
468+
)
469+
)
470+
viewStore.send(())
471+
DispatchQueue.global().asyncAfter(deadline: .now() + 0.1) {
472+
viewStore.send(())
473+
}
474+
}
475+
}
476+
477+
wait(for: expectations, timeout: 1)
478+
}
450479
}

0 commit comments

Comments
 (0)