From 071d5b513b52f3b1f9876f54543301a5883f4f67 Mon Sep 17 00:00:00 2001 From: Philippe Hausler Date: Tue, 17 Jun 2025 11:21:12 -0700 Subject: [PATCH 1/2] [Observation] Restrict deinitialization fired events to be limited to one per registrar and send a \Self.self as the keypath to avoid property confusions --- .../Observation/ObservationRegistrar.swift | 26 +++++++++++-------- test/stdlib/Observation/Observable.swift | 8 +++--- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/stdlib/public/Observation/Sources/Observation/ObservationRegistrar.swift b/stdlib/public/Observation/Sources/Observation/ObservationRegistrar.swift index fd61a2d28fc5a..b00f5288c34a0 100644 --- a/stdlib/public/Observation/Sources/Observation/ObservationRegistrar.swift +++ b/stdlib/public/Observation/Sources/Observation/ObservationRegistrar.swift @@ -110,20 +110,27 @@ public struct ObservationRegistrar: Sendable { } } - internal mutating func deinitialize() -> [@Sendable () -> Void] { - var trackers = [@Sendable () -> Void]() + internal mutating func deinitialize() -> (@Sendable () -> Void)? { + func extractSelf(_ ty: T.Type) -> AnyKeyPath { + return \T.self + } + + var tracker: (@Sendable () -> Void)? for (keyPath, ids) in lookups { for id in ids { - if let tracker = observations[id]?.willSetTracker { - trackers.append({ - tracker(keyPath) - }) + if let found = observations[id]?.willSetTracker { + // convert the keyPath into its \Self.self version + let selfKp = _openExistential(type(of: keyPath).rootType, do: extractSelf) + tracker = { + found(selfKp) + } + break } } } observations.removeAll() lookups.removeAll() - return trackers + return tracker } internal mutating func willSet(keyPath: AnyKeyPath) -> [@Sendable (AnyKeyPath) -> Void] { @@ -169,10 +176,7 @@ public struct ObservationRegistrar: Sendable { } internal func deinitialize() { - let tracking = state.withCriticalRegion { $0.deinitialize() } - for action in tracking { - action() - } + state.withCriticalRegion { $0.deinitialize() }?() } internal func willSet( diff --git a/test/stdlib/Observation/Observable.swift b/test/stdlib/Observation/Observable.swift index 2f05994060f75..81b3f7bf1cbc6 100644 --- a/test/stdlib/Observation/Observable.swift +++ b/test/stdlib/Observation/Observable.swift @@ -290,6 +290,7 @@ final class CowTest { @Observable final class DeinitTriggeredObserver { var property: Int = 3 + var property2: Int = 4 let deinitTrigger: () -> Void init(_ deinitTrigger: @escaping () -> Void) { @@ -528,17 +529,18 @@ struct Validator { suite.test("weak container observation") { let changed = CapturedState(state: false) - let deinitialized = CapturedState(state: false) + let deinitialized = CapturedState(state: 0) var test = DeinitTriggeredObserver { - deinitialized.state = true + deinitialized.state += 1 } withObservationTracking { [weak test] in _blackHole(test?.property) + _blackHole(test?.property2) } onChange: { changed.state = true } test = DeinitTriggeredObserver { } - expectEqual(deinitialized.state, true) + expectEqual(deinitialized.state, 1) // ensure only one invocation is done per deinitialization expectEqual(changed.state, true) } From 9f39256c9b3ca744f95b4f324084d02aedc64366 Mon Sep 17 00:00:00 2001 From: Philippe Hausler Date: Wed, 2 Jul 2025 15:21:05 -0700 Subject: [PATCH 2/2] Break the outer loop of iteration for deinitialization tracker --- .../Sources/Observation/ObservationRegistrar.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stdlib/public/Observation/Sources/Observation/ObservationRegistrar.swift b/stdlib/public/Observation/Sources/Observation/ObservationRegistrar.swift index b00f5288c34a0..58fd1ff827bf4 100644 --- a/stdlib/public/Observation/Sources/Observation/ObservationRegistrar.swift +++ b/stdlib/public/Observation/Sources/Observation/ObservationRegistrar.swift @@ -116,7 +116,7 @@ public struct ObservationRegistrar: Sendable { } var tracker: (@Sendable () -> Void)? - for (keyPath, ids) in lookups { + lookupIteration: for (keyPath, ids) in lookups { for id in ids { if let found = observations[id]?.willSetTracker { // convert the keyPath into its \Self.self version @@ -124,7 +124,7 @@ public struct ObservationRegistrar: Sendable { tracker = { found(selfKp) } - break + break lookupIteration } } }