Skip to content

Commit a349e64

Browse files
authored
[Observation] Ensure deinitialized Observable types don't leave active observations in memory (#82752)
Explanation: This ensures a potential leak with SwiftUI and other systems using Observation do not leak observation closures when the potential Observable instances used are only weakly referenced inside the tracking closure. Scope: This is limited to the runtime behavior of Observable types and has no ABI or language level interactions. Issues: rdar://112167556 Original PRs: #79823 #82307 Risk: Low - This is very targeted to just Observation, however it is a behavioral change which does not make this a zero risk change. Testing: New unit tests were added to catch at least some of the potential cases this issue can occur with.
1 parent 1dd4a99 commit a349e64

File tree

2 files changed

+55
-4
lines changed

2 files changed

+55
-4
lines changed

stdlib/public/Observation/Sources/Observation/ObservationRegistrar.swift

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,27 @@ public struct ObservationRegistrar: Sendable {
110110
}
111111
}
112112

113-
internal mutating func cancelAll() {
113+
internal mutating func deinitialize() -> (@Sendable () -> Void)? {
114+
func extractSelf<T>(_ ty: T.Type) -> AnyKeyPath {
115+
return \T.self
116+
}
117+
118+
var tracker: (@Sendable () -> Void)?
119+
lookupIteration: for (keyPath, ids) in lookups {
120+
for id in ids {
121+
if let found = observations[id]?.willSetTracker {
122+
// convert the keyPath into its \Self.self version
123+
let selfKp = _openExistential(type(of: keyPath).rootType, do: extractSelf)
124+
tracker = {
125+
found(selfKp)
126+
}
127+
break lookupIteration
128+
}
129+
}
130+
}
114131
observations.removeAll()
115132
lookups.removeAll()
133+
return tracker
116134
}
117135

118136
internal mutating func willSet(keyPath: AnyKeyPath) -> [@Sendable (AnyKeyPath) -> Void] {
@@ -157,8 +175,8 @@ public struct ObservationRegistrar: Sendable {
157175
state.withCriticalRegion { $0.cancel(id) }
158176
}
159177

160-
internal func cancelAll() {
161-
state.withCriticalRegion { $0.cancelAll() }
178+
internal func deinitialize() {
179+
state.withCriticalRegion { $0.deinitialize() }?()
162180
}
163181

164182
internal func willSet<Subject: Observable, Member>(
@@ -189,7 +207,7 @@ public struct ObservationRegistrar: Sendable {
189207
}
190208

191209
deinit {
192-
context.cancelAll()
210+
context.deinitialize()
193211
}
194212
}
195213

test/stdlib/Observation/Observable.swift

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,22 @@ final class CowTest {
287287
var container = CowContainer()
288288
}
289289

290+
@Observable
291+
final class DeinitTriggeredObserver {
292+
var property: Int = 3
293+
var property2: Int = 4
294+
let deinitTrigger: () -> Void
295+
296+
init(_ deinitTrigger: @escaping () -> Void) {
297+
self.deinitTrigger = deinitTrigger
298+
}
299+
300+
deinit {
301+
deinitTrigger()
302+
}
303+
}
304+
305+
290306
@main
291307
struct Validator {
292308
@MainActor
@@ -511,6 +527,23 @@ struct Validator {
511527
expectEqual(subject.container.id, startId)
512528
}
513529

530+
suite.test("weak container observation") {
531+
let changed = CapturedState(state: false)
532+
let deinitialized = CapturedState(state: 0)
533+
var test = DeinitTriggeredObserver {
534+
deinitialized.state += 1
535+
}
536+
withObservationTracking { [weak test] in
537+
_blackHole(test?.property)
538+
_blackHole(test?.property2)
539+
} onChange: {
540+
changed.state = true
541+
}
542+
test = DeinitTriggeredObserver { }
543+
expectEqual(deinitialized.state, 1) // ensure only one invocation is done per deinitialization
544+
expectEqual(changed.state, true)
545+
}
546+
514547
runAllTests()
515548
}
516549
}

0 commit comments

Comments
 (0)