Skip to content

Commit 3eff869

Browse files
authored
fix(store): prevent stale state listeners from crashing apps (#119)
* fix(store): prevent stale state listeners from crashing apps * fix(store): improve code maintenance
1 parent 94b2fc7 commit 3eff869

3 files changed

Lines changed: 313 additions & 58 deletions

File tree

Sources/Rownd/Models/Context/ReSwiftObserver.swift

Lines changed: 64 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,17 @@ import Foundation
1010
import ReSwift
1111
import SwiftUI
1212

13+
// MARK: - Main Thread Dispatch Helper
14+
15+
/// Helper to centralize main-thread dispatch with weak self handling.
16+
/// Reduces duplication and ensures consistent patterns across observable state types.
17+
private func dispatchOnMain<T: AnyObject>(_ instance: T, execute work: @escaping (T) -> Void) {
18+
DispatchQueue.main.async { [weak instance] in
19+
guard let instance = instance else { return }
20+
work(instance)
21+
}
22+
}
23+
1324
public class ObservableState<T: Hashable>: ObservableObject, StoreSubscriber, ObservableSubscription
1425
{
1526

@@ -31,22 +42,22 @@ public class ObservableState<T: Hashable>: ObservableObject, StoreSubscriber, Ob
3142

3243
public func subscribe() {
3344
guard !isSubscribed else { return }
34-
DispatchQueue.main.async { [weak self] in
35-
guard let self = self else { return }
36-
guard !self.isSubscribed else { return }
45+
// Capture selector directly to avoid retaining self in the transform closure
46+
let selector = self.selector
47+
dispatchOnMain(self) { instance in
48+
guard !instance.isSubscribed else { return }
3749
Context.currentContext.store.subscribe(
38-
self, transform: { [self] in $0.select(self.selector) })
39-
self.isSubscribed = true
50+
instance, transform: { $0.select(selector) })
51+
instance.isSubscribed = true
4052
}
4153
}
4254

4355
func unsubscribe() {
4456
guard isSubscribed else { return }
45-
DispatchQueue.main.async { [weak self] in
46-
guard let self = self else { return }
47-
guard self.isSubscribed else { return }
48-
Context.currentContext.store.unsubscribe(self)
49-
self.isSubscribed = false
57+
dispatchOnMain(self) { instance in
58+
guard instance.isSubscribed else { return }
59+
Context.currentContext.store.unsubscribe(instance)
60+
instance.isSubscribed = false
5061
}
5162
}
5263

@@ -55,17 +66,18 @@ public class ObservableState<T: Hashable>: ObservableObject, StoreSubscriber, Ob
5566
}
5667

5768
public func newState(state: T) {
58-
guard self.current != state else { return }
59-
DispatchQueue.main.async {
60-
let old = self.current
61-
if let animation = self.animation {
69+
// All @Published property access must happen on main thread
70+
dispatchOnMain(self) { instance in
71+
guard instance.current != state else { return }
72+
let old = instance.current
73+
if let animation = instance.animation {
6274
withAnimation(animation) {
63-
self.current = state
75+
instance.current = state
6476
}
6577
} else {
66-
self.current = state
78+
instance.current = state
6779
}
68-
self.objectDidChange.send(DidChangeSubject(old: old, new: self.current))
80+
instance.objectDidChange.send(DidChangeSubject(old: old, new: instance.current))
6981
}
7082
}
7183

@@ -94,17 +106,19 @@ public class ObservableThrottledState<T: Hashable>: ObservableState<T> {
94106
}
95107

96108
override public func newState(state: T) {
97-
guard self.current != state else { return }
98-
DispatchQueue.main.async {
99-
let old = self.current
100-
if let animation = self.animation {
109+
// All @Published property access must happen on main thread to avoid crashes
110+
// in swift_retain when accessing Combine's Published wrapper from background threads
111+
dispatchOnMain(self) { instance in
112+
guard instance.current != state else { return }
113+
let old = instance.current
114+
if let animation = instance.animation {
101115
withAnimation(animation) {
102-
self.objectThrottled.send(state)
116+
instance.objectThrottled.send(state)
103117
}
104118
} else {
105-
self.objectThrottled.send(state)
119+
instance.objectThrottled.send(state)
106120
}
107-
self.objectDidChange.send(DidChangeSubject(old: old, new: self.current))
121+
instance.objectDidChange.send(DidChangeSubject(old: old, new: instance.current))
108122
}
109123
}
110124

@@ -137,22 +151,22 @@ public class ObservableDerivedState<Original: Hashable, Derived: Hashable>: Obse
137151

138152
func subscribe() {
139153
guard !isSubscribed else { return }
140-
DispatchQueue.main.async { [weak self] in
141-
guard let self = self else { return }
142-
guard !self.isSubscribed else { return }
154+
// Capture selector directly to avoid retaining self in the transform closure
155+
let selector = self.selector
156+
dispatchOnMain(self) { instance in
157+
guard !instance.isSubscribed else { return }
143158
Context.currentContext.store.subscribe(
144-
self, transform: { [self] in $0.select(self.selector) })
145-
self.isSubscribed = true
159+
instance, transform: { $0.select(selector) })
160+
instance.isSubscribed = true
146161
}
147162
}
148163

149164
func unsubscribe() {
150165
guard isSubscribed else { return }
151-
DispatchQueue.main.async { [weak self] in
152-
guard let self = self else { return }
153-
guard self.isSubscribed else { return }
154-
Context.currentContext.store.unsubscribe(self)
155-
self.isSubscribed = false
166+
dispatchOnMain(self) { instance in
167+
guard instance.isSubscribed else { return }
168+
Context.currentContext.store.unsubscribe(instance)
169+
instance.isSubscribed = false
156170
}
157171
}
158172

@@ -161,18 +175,18 @@ public class ObservableDerivedState<Original: Hashable, Derived: Hashable>: Obse
161175
}
162176

163177
public func newState(state original: Original) {
164-
DispatchQueue.main.async {
165-
let old = self.current
166-
self.objectWillChange.send(ChangeSubject(old: old, new: self.current))
178+
dispatchOnMain(self) { instance in
179+
let old = instance.current
180+
instance.objectWillChange.send(ChangeSubject(old: old, new: instance.current))
167181

168-
if let animation = self.animation {
182+
if let animation = instance.animation {
169183
withAnimation(animation) {
170-
self.current = self.transform(original)
184+
instance.current = instance.transform(original)
171185
}
172186
} else {
173-
self.current = self.transform(original)
187+
instance.current = instance.transform(original)
174188
}
175-
self.objectDidChange.send(ChangeSubject(old: old, new: self.current))
189+
instance.objectDidChange.send(ChangeSubject(old: old, new: instance.current))
176190
}
177191
}
178192

@@ -207,17 +221,16 @@ public class ObservableDerivedThrottledState<Original: Hashable, Derived: Hashab
207221
}
208222

209223
override public func newState(state original: Original) {
210-
let old = current
211-
if let animation = animation {
212-
withAnimation(animation) {
213-
objectThrottled.send(original)
224+
dispatchOnMain(self) { instance in
225+
let old = instance.current
226+
if let animation = instance.animation {
227+
withAnimation(animation) {
228+
instance.objectThrottled.send(original)
229+
}
230+
} else {
231+
instance.objectThrottled.send(original)
214232
}
215-
} else {
216-
objectThrottled.send(original)
217-
}
218-
219-
DispatchQueue.main.async {
220-
self.objectDidChange.send(ChangeSubject(old: old, new: self.current))
233+
instance.objectDidChange.send(ChangeSubject(old: old, new: instance.current))
221234
}
222235
}
223236

0 commit comments

Comments
 (0)