Skip to content

Commit d080493

Browse files
mluisbrownmluisbrown
committed
Update to thread warnings. (#773)
* Update to thread warnings. * fix tests * clean up * rename variable
1 parent 0e2be7e commit d080493

File tree

3 files changed

+124
-77
lines changed

3 files changed

+124
-77
lines changed

‎Sources/ComposableArchitecture/Store.swift

Lines changed: 63 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,9 @@ public final class Store<State, Action> {
129129
private var parentDisposable: Disposable?
130130
private let reducer: (inout State, Action) -> Effect<Action, Never>
131131
private var bufferedActions: [Action] = []
132+
#if DEBUG
133+
private var initialThread = Thread.current
134+
#endif
132135

133136
/// Initializes a store from an initial state, a reducer, and an environment.
134137
///
@@ -372,7 +375,9 @@ public final class Store<State, Action> {
372375
self.producerScope(state: toLocalState, action: { $0 })
373376
}
374377

375-
func send(_ action: Action) {
378+
func send(_ action: Action, isFromViewStore: Bool = true) {
379+
self.threadCheck(status: .send(action, isFromViewStore: isFromViewStore))
380+
376381
self.bufferedActions.append(action)
377382
guard !self.isSending else { return }
378383

@@ -389,43 +394,12 @@ public final class Store<State, Action> {
389394

390395
var didComplete = false
391396
let uuid = UUID()
392-
393-
#if DEBUG
394-
let initalThread = Thread.current
395-
initalThread.threadDictionary[uuid] = true
396-
#endif
397-
398397
let observer = Signal<Action, Never>.Observer(
399398
value: { [weak self] action in
400-
self?.send(action)
399+
self?.send(action, isFromViewStore: false)
401400
},
402401
completed: { [weak self] in
403-
#if DEBUG
404-
if Thread.current.threadDictionary[uuid] == nil {
405-
breakpoint(
406-
"""
407-
---
408-
Warning: Store.send
409-
410-
The Store class is not thread-safe, and so all interactions with an instance of Store
411-
(including all of its scopes and derived ViewStores) must be done on the same thread.
412-
413-
\(debugCaseOutput(action)) has produced an Effect that was completed on a different thread \
414-
from the one it was executed on.
415-
416-
Starting thread: \(initalThread)
417-
Final thread: \(Thread.current)
418-
419-
Possible fixes for this are:
420-
421-
* Add a .receive(on:) to the Effect to ensure it completes on this Stores correct thread.
422-
"""
423-
)
424-
}
425-
426-
Thread.current.threadDictionary[uuid] = nil
427-
#endif
428-
402+
self?.threadCheck(status: .effectCompletion(action))
429403
didComplete = true
430404
self?.effectDisposables.removeValue(forKey: uuid)?.dispose()
431405
},
@@ -459,4 +433,59 @@ public final class Store<State, Action> {
459433
self.effectDisposables.removeValue(forKey: id)?.dispose()
460434
}
461435
}
436+
437+
private enum ThreadCheckStatus {
438+
case effectCompletion(Action)
439+
case send(Action, isFromViewStore: Bool)
440+
}
441+
442+
@inline(__always)
443+
private func threadCheck(status: ThreadCheckStatus) {
444+
#if DEBUG
445+
guard self.initialThread != Thread.current
446+
else { return }
447+
448+
let message: String
449+
switch status {
450+
case let .effectCompletion(action):
451+
message = """
452+
An effect returned from the action "\(debugCaseOutput(action))" completed on the \
453+
wrong thread. Make sure to use ".receive(on:)" on any effects that execute on background \
454+
threads to receive their output on the initial thread.
455+
"""
456+
457+
case let .send(action, isFromViewStore: true):
458+
message = """
459+
"ViewStore.send(\(debugCaseOutput(action)))" was called on the wrong thread. Make \
460+
sure that "ViewStore.send" is always called on the initial thread.
461+
"""
462+
463+
case let .send(action, isFromViewStore: false):
464+
message = """
465+
An effect emitted the action "\(debugCaseOutput(action))" from the wrong thread. Make sure \
466+
to use ".receive(on:)" on any effects that execute on background threads to receive their \
467+
output on the initial thread.
468+
"""
469+
}
470+
471+
breakpoint(
472+
"""
473+
---
474+
Warning:
475+
476+
The store was interacted with on a thread that is different from the thread the store was \
477+
created on:
478+
479+
\(message)
480+
481+
Initial thread: \(self.initialThread)
482+
Current thread: \(Thread.current)
483+
484+
The "Store" class is not thread-safe, and so all interactions with an instance of "Store" \
485+
(including all of its scopes and derived view stores) must be done on the same thread.
486+
---
487+
"""
488+
)
489+
#endif
490+
}
462491
}

‎Sources/ComposableArchitecture/ViewStore.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,8 @@ public final class ViewStore<State, Action> {
8585
_ store: Store<State, Action>,
8686
removeDuplicates isDuplicate: @escaping (State, State) -> Bool
8787
) {
88+
self._send = { store.send($0) }
8889
self._state = store.state
89-
self._send = store.send
9090

9191
self.viewDisposable = store.producer
9292
.skipRepeats(isDuplicate)
@@ -100,7 +100,7 @@ public final class ViewStore<State, Action> {
100100
#endif
101101
self._state = $0
102102
self.statePipe.input.send(value: $0)
103-
}
103+
}
104104
}
105105

106106
/// A `SignalProducerConvertible` that emits when state changes.
@@ -295,7 +295,7 @@ public final class ViewStore<State, Action> {
295295

296296
deinit {
297297
viewDisposable?.dispose()
298-
}
298+
}
299299
}
300300

301301
extension ViewStore where State: Equatable {

‎Tests/ComposableArchitectureTests/DebugTests.swift

Lines changed: 58 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ final class DebugTests: XCTestCase {
348348

349349
#if canImport(SwiftUI)
350350
@available(iOS 13, macOS 10.15, macCatalyst 13, tvOS 13, watchOS 6, *)
351-
func testTextState() {
351+
func testTextState() {
352352
XCTAssertEqual(
353353
debugOutput(
354354
TextState("Hello, world!")
@@ -358,60 +358,60 @@ final class DebugTests: XCTestCase {
358358
Hello, world!
359359
)
360360
"""
361-
)
361+
)
362362

363363
XCTAssertEqual(
364364
debugOutput(
365-
TextState("Hello, ")
366-
+ TextState("world").bold().italic()
365+
TextState("Hello, ")
366+
+ TextState("world").bold().italic()
367367
+ TextState("!")
368368
),
369369
"""
370370
TextState(
371371
Hello, _**world**_!
372372
)
373373
"""
374-
)
374+
)
375375

376376
XCTAssertEqual(
377377
debugOutput(
378-
TextState("Offset by 10.5").baselineOffset(10.5)
379-
+ TextState("\n") + TextState("Headline").font(.headline)
380-
+ TextState("\n") + TextState("No font").font(nil)
381-
+ TextState("\n") + TextState("Light font weight").fontWeight(.light)
382-
+ TextState("\n") + TextState("No font weight").fontWeight(nil)
383-
+ TextState("\n") + TextState("Red").foregroundColor(.red)
384-
+ TextState("\n") + TextState("No color").foregroundColor(nil)
385-
+ TextState("\n") + TextState("Italic").italic()
386-
+ TextState("\n") + TextState("Kerning of 2.5").kerning(2.5)
387-
+ TextState("\n") + TextState("Stricken").strikethrough()
388-
+ TextState("\n") + TextState("Stricken green").strikethrough(color: .green)
389-
+ TextState("\n") + TextState("Not stricken blue").strikethrough(false, color: .blue)
390-
+ TextState("\n") + TextState("Tracking of 5.5").tracking(5.5)
391-
+ TextState("\n") + TextState("Underlined").underline()
392-
+ TextState("\n") + TextState("Underlined pink").underline(color: .pink)
378+
TextState("Offset by 10.5").baselineOffset(10.5)
379+
+ TextState("\n") + TextState("Headline").font(.headline)
380+
+ TextState("\n") + TextState("No font").font(nil)
381+
+ TextState("\n") + TextState("Light font weight").fontWeight(.light)
382+
+ TextState("\n") + TextState("No font weight").fontWeight(nil)
383+
+ TextState("\n") + TextState("Red").foregroundColor(.red)
384+
+ TextState("\n") + TextState("No color").foregroundColor(nil)
385+
+ TextState("\n") + TextState("Italic").italic()
386+
+ TextState("\n") + TextState("Kerning of 2.5").kerning(2.5)
387+
+ TextState("\n") + TextState("Stricken").strikethrough()
388+
+ TextState("\n") + TextState("Stricken green").strikethrough(color: .green)
389+
+ TextState("\n") + TextState("Not stricken blue").strikethrough(false, color: .blue)
390+
+ TextState("\n") + TextState("Tracking of 5.5").tracking(5.5)
391+
+ TextState("\n") + TextState("Underlined").underline()
392+
+ TextState("\n") + TextState("Underlined pink").underline(color: .pink)
393393
+ TextState("\n") + TextState("Not underlined purple").underline(false, color: .pink)
394394
),
395-
"""
396-
TextState(
397-
<baseline-offset=10.5>Offset by 10.5</baseline-offset>
398-
Headline
399-
No font
400-
<font-weight=light>Light font weight</font-weight>
401-
No font weight
402-
<foreground-color=red>Red</foreground-color>
403-
No color
404-
_Italic_
405-
<kerning=2.5>Kerning of 2.5</kerning>
406-
~~Stricken~~
407-
<s color=green>Stricken green</s>
408-
Not stricken blue
409-
<tracking=5.5>Tracking of 5.5</tracking>
410-
<u>Underlined</u>
411-
<u color=pink>Underlined pink</u>
412-
Not underlined purple
413-
)
414-
"""
395+
"""
396+
TextState(
397+
<baseline-offset=10.5>Offset by 10.5</baseline-offset>
398+
Headline
399+
No font
400+
<font-weight=light>Light font weight</font-weight>
401+
No font weight
402+
<foreground-color=red>Red</foreground-color>
403+
No color
404+
_Italic_
405+
<kerning=2.5>Kerning of 2.5</kerning>
406+
~~Stricken~~
407+
<s color=green>Stricken green</s>
408+
Not stricken blue
409+
<tracking=5.5>Tracking of 5.5</tracking>
410+
<u>Underlined</u>
411+
<u color=pink>Underlined pink</u>
412+
Not underlined purple
413+
)
414+
"""
415415
)
416416
}
417417
#endif
@@ -960,4 +960,22 @@ final class DebugTests: XCTestCase {
960960
"""
961961
)
962962
}
963+
964+
func testBindingAction() {
965+
struct State {
966+
@BindableState var width = 0
967+
}
968+
969+
XCTAssertEqual(
970+
debugOutput(BindingAction.set(\State.$width, 50)),
971+
"""
972+
BindingAction<State>(
973+
keyPath: WritableKeyPath<State, BindableState<Int>>(),
974+
set: (Function),
975+
value: 50,
976+
valueIsEqualTo: (Function)
977+
)
978+
"""
979+
)
980+
}
963981
}

0 commit comments

Comments
 (0)