Просмотр исходного кода

Drop weak observer cache in BolusProgressState

setReporter took the observer as a parameter but clear() reached for a
weak cached copy. If BaseAPSManager were torn down between the two
calls, the weak ref would be nil and removeObserver would be skipped —
leaving the observer in whatever internal list DoseProgressReporter
keeps and leaking it through that ref.

Holding it strong inside the actor isn't an option either: that forms
a retain cycle (BaseAPSManager → state → observer = BaseAPSManager).

Fix: drop the cache entirely. Both setReporter and clear now take
observer as a parameter — the caller (BaseAPSManager) always has a
stable self to pass, and Swift retains it across the await for the
duration of the actor call without forming a long-lived cycle.
Marvin Polscheit 1 день назад
Родитель
Сommit
123e552814
1 измененных файлов с 9 добавлено и 5 удалено
  1. 9 5
      Trio/Sources/APS/APSManager.swift

+ 9 - 5
Trio/Sources/APS/APSManager.swift

@@ -101,20 +101,24 @@ private actor LoopGuard {
 /// data races between Combine sinks (processQueue) and async contexts (cancelBolus).
 private actor BolusProgressState {
     private var reporter: DoseProgressReporter?
-    private weak var observer: (any DoseProgressObserver)?
     private var generation: UInt64 = 0
 
+    // Observer is supplied by the caller on every call instead of being cached weakly:
+    // a stored weak ref could nil out between setReporter and clear (if the owning
+    // BaseAPSManager were torn down in between), making the removeObserver call a no-op
+    // and leaking the observer through any internal strong list inside DoseProgressReporter.
+    // Holding it strong here would form a retain cycle (BaseAPSManager → state → observer).
+
     func setReporter(_ newReporter: DoseProgressReporter?, observer: any DoseProgressObserver) {
         generation &+= 1
         reporter?.removeObserver(observer)
         reporter = newReporter
         reporter?.addObserver(observer)
-        self.observer = observer
     }
 
-    func clear() -> UInt64 {
+    func clear(observer: any DoseProgressObserver) -> UInt64 {
         generation &+= 1
-        if let observer { reporter?.removeObserver(observer) }
+        reporter?.removeObserver(observer)
         reporter = nil
         return generation
     }
@@ -1229,7 +1233,7 @@ final class BaseAPSManager: APSManager, Injectable {
     }
 
     private func clearBolusReporter() async {
-        let token = await bolusProgressState.clear()
+        let token = await bolusProgressState.clear(observer: self)
         try? await Task.sleep(for: .milliseconds(500))
         // Generation token guards against a new bolus starting during the 500 ms grace:
         // if `setReporter` ran in between, our stale `send(nil)` would clobber its initial `send(0)`.