Skip to content

Commit bcdc8c1

Browse files
nolanwjessesquires
andauthored
Fix crash dereferencing unowned _publisher in KVO trampoline (#112)
A `FoilDefaultStorage{,Optional}` can deallocate while its `ObserverTrampoline` sticks around long enough to receive a KVO notification. (I'm not sure how.) When this happens, the attempt to dereference the captured `_publisher` crashes. I don't know how to write a reliable test for the test suite, but I can reproduce the crash ~instantly with this code (e.g. pasted into a new test) in Xcode 16.0 beta 5 on an iPhone 8 running iOS 16.7.8 (as well as in iOS 15, 16, 17, and 18 simulators I have kicking around): ```swift DispatchQueue.global().async { var storage: FoilDefaultStorageOptional<Int>? while true { storage = .init(key: "aloha") } } while true { UserDefaults.standard.set(Int.random(in: Int.min...Int.max), forKey: "aloha") } ``` That's an admittedly silly test, but I'm seeing crashes in the wild attempting to dereference `_publisher`. The obvious fix to me is to capture `_publisher` as either weak or strong, instead of unowned. A strong reference doesn't cause a cycle, so I chose it over the theoretically slower weak reference. --------- Co-authored-by: Jesse Squires <[email protected]>
1 parent 8ed1689 commit bcdc8c1

File tree

3 files changed

+13
-6
lines changed

3 files changed

+13
-6
lines changed

CHANGELOG.md

+5
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ NEXT
77

88
- TBA
99

10+
5.1.2
11+
-----
12+
13+
- Fixed possible crash when receiving a KVO notification and so attempting to send a value to `FoilDefaultStorage` or `FoilDefaultStorageOptional`'s publisher after the storage is deallocated. ([#112](https://github.com/jessesquires/Foil/issues/112), [@nolanw](https://github.com/nolanw))
14+
1015
5.1.1
1116
-----
1217

Sources/FoilDefaultStorage.swift

+4-3
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,11 @@ public struct FoilDefaultStorage<T: UserDefaultsSerializable> {
5353
// Publisher must be initialized after `registerDefault`,
5454
// because `fetch` assumes that `registerDefault` has been called before
5555
// and uses force unwrap
56-
self._publisher = CurrentValueSubject<T, Never>(userDefaults.fetch(keyName))
56+
let publisher = CurrentValueSubject<T, Never>(userDefaults.fetch(keyName))
57+
self._publisher = publisher
5758

58-
self._observer = ObserverTrampoline(userDefaults: userDefaults, key: keyName) { [unowned _publisher] in
59-
_publisher.send(userDefaults.fetch(keyName))
59+
self._observer = ObserverTrampoline(userDefaults: userDefaults, key: keyName) {
60+
publisher.send(userDefaults.fetch(keyName))
6061
}
6162
}
6263
}

Sources/FoilDefaultStorageOptional.swift

+4-3
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,10 @@ public struct FoilDefaultStorageOptional<T: UserDefaultsSerializable> {
5151
public init(key keyName: String, userDefaults: UserDefaults = .standard) {
5252
self.key = keyName
5353
self._userDefaults = userDefaults
54-
self._publisher = CurrentValueSubject<T?, Never>(userDefaults.fetchOptional(keyName))
55-
self._observer = ObserverTrampoline(userDefaults: userDefaults, key: keyName) { [unowned _publisher] in
56-
_publisher.send(userDefaults.fetchOptional(keyName))
54+
let publisher = CurrentValueSubject<T?, Never>(userDefaults.fetchOptional(keyName))
55+
self._publisher = publisher
56+
self._observer = ObserverTrampoline(userDefaults: userDefaults, key: keyName) {
57+
publisher.send(userDefaults.fetchOptional(keyName))
5758
}
5859
}
5960
}

0 commit comments

Comments
 (0)