From 546a66d193414100209368f7a05ba874f394e328 Mon Sep 17 00:00:00 2001 From: iBobby Date: Tue, 20 Oct 2026 12:40:55 -0600 Subject: [PATCH] app: quiesce the fan pipeline and drain the pump before the exit resetAuto MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit applicationWillTerminate ran its synchronous resetAuto while the monitor was still ticking and the command pump could still hold an in-flight or pending write. That write lands at the daemon AFTER the exit resetAuto and re-pins the fans in manual — with the app gone, they stay pinned until the daemon watchdog reverts them. The quit path now quiesces first, in order: stop the monitor's ticks and reject new submits (a terminating flag guards the onFanCommand hop), then flush() — a new synchronous barrier on the pump's queue that blocks until every command submitted before it has executed. Only then does the exit resetAuto run, so nothing can supersede it. The AppDelegate holds the AppState reference (handed over in ThermalForgeApp.init) to reach the pipeline at quit time. Two pump tests cover the barrier: flush returns only after in-flight and pending writes have executed, and the pump still serves submits made after a flush. --- Sources/ThermalForgeApp/AppState.swift | 25 +++++++++++++- Sources/ThermalForgeApp/ThermalForgeApp.swift | 24 +++++++++++++- Sources/ThermalForgeCore/FanCommandPump.swift | 14 ++++++++ .../FanCommandPumpTests.swift | 33 +++++++++++++++++++ 4 files changed, 94 insertions(+), 2 deletions(-) diff --git a/Sources/ThermalForgeApp/AppState.swift b/Sources/ThermalForgeApp/AppState.swift index cd15b5c4..d5f750af 100644 --- a/Sources/ThermalForgeApp/AppState.swift +++ b/Sources/ThermalForgeApp/AppState.swift @@ -48,6 +48,10 @@ final class AppState: ObservableObject { @Published var availableUpdate: AvailableUpdate? private var monitor: ThermalMonitor? + /// Set by prepareForTermination: once true, no new fan command may enter + /// the pump (the exit path is about to drain and reset). Guards the quit + /// ordering, not UI state. + private var terminating = false private let executor = PrivilegedExecutor() private var heartbeatTimer: DispatchSourceTimer? /// Consecutive failed heartbeats, for debouncing `daemonUnreachable`. @@ -362,7 +366,10 @@ final class AppState: ObservableObject { // Don't fight a CLI hold — the user set it deliberately. Decide on // the main actor where externalHold lives; the monitor resumes // control when they pick a profile or press Default. - guard self.externalHold == nil else { return } + // terminating: the quit path is draining the pump — a late + // submit would land after the exit resetAuto and re-pin the + // fans. + guard self.externalHold == nil, !self.terminating else { return } // Hand off to the coalescing pump; the blocking socket write happens // OFF the main thread. During a ramp these fire up to ~10x/sec; // previously each ran a blocking round-trip on the main actor and @@ -374,6 +381,22 @@ final class AppState: ObservableObject { self.monitor = monitor } + // MARK: - Termination + + /// Quiesce the fan pipeline for app termination. Order matters: reject new + /// submits and stop the monitor's ticks FIRST, then drain the pump — so + /// the synchronous resetAuto AppDelegate runs after this returns can never + /// be superseded by a late-landing ramp write (an observed quit race: the + /// last monitor tick's setRPM reached the daemon AFTER the exit resetAuto + /// and re-pinned the fans in manual until the daemon watchdog reverted + /// them). + func prepareForTermination() { + terminating = true + monitor?.stop() + monitor = nil + commandPump.flush() + } + // MARK: - Actions /// Explicit user takeover of any reflected CLI hold. Returns whether one was diff --git a/Sources/ThermalForgeApp/ThermalForgeApp.swift b/Sources/ThermalForgeApp/ThermalForgeApp.swift index 453e5b20..195f462e 100644 --- a/Sources/ThermalForgeApp/ThermalForgeApp.swift +++ b/Sources/ThermalForgeApp/ThermalForgeApp.swift @@ -9,6 +9,12 @@ import SwiftUI import ThermalForgeCore class AppDelegate: NSObject, NSApplicationDelegate { + /// Held (handed over by ThermalForgeApp.init) so applicationWillTerminate + /// can quiesce the fan pipeline — otherwise the exit resetAuto below runs + /// while the monitor's pipeline still has an in-flight write, which can + /// land a moment later and re-pin the fans in manual. + @MainActor var appState: AppState? + func applicationDidFinishLaunching(_ notification: Notification) { // No Dock icon — menu bar only NSApp.setActivationPolicy(.accessory) @@ -23,6 +29,14 @@ class AppDelegate: NSObject, NSApplicationDelegate { } func applicationWillTerminate(_ notification: Notification) { + // Quiesce FIRST: stop the monitor's ticks, reject new submits, and + // drain the pump — otherwise an in-flight ramp write can land at the + // daemon AFTER the resetAuto below and re-pin the fans in manual + // until the daemon watchdog reverts them. + MainActor.assumeIsolated { + appState?.prepareForTermination() + } + // Reset fans on quit so the daemon doesn't hold stale APP settings — but // ONLY if the app owns the hold. A CLI hold (`sudo thermalforge max`) is the // user's deliberate, unsupervised choice; quitting the menu bar app must not @@ -40,7 +54,15 @@ class AppDelegate: NSObject, NSApplicationDelegate { @main struct ThermalForgeApp: App { @NSApplicationDelegateAdaptor(AppDelegate.self) var delegate - @StateObject private var appState = AppState() + @StateObject private var appState: AppState + + init() { + let state = AppState() + _appState = StateObject(wrappedValue: state) + // The StateObject owns it for SwiftUI; the delegate holds the same + // instance so the quit path can quiesce the fan pipeline. + delegate.appState = state + } var body: some Scene { MenuBarExtra { diff --git a/Sources/ThermalForgeCore/FanCommandPump.swift b/Sources/ThermalForgeCore/FanCommandPump.swift index 130a7813..857b5ff4 100644 --- a/Sources/ThermalForgeCore/FanCommandPump.swift +++ b/Sources/ThermalForgeCore/FanCommandPump.swift @@ -88,4 +88,18 @@ public final class FanCommandPump: @unchecked Sendable { done?(ok) } } + + /// Synchronous barrier: blocks until every command submitted BEFORE this + /// call has executed. Termination ordering: the exit path stops new + /// submits, flushes, then runs its resetAuto on a separate connection — + /// without the barrier an in-flight write can land at the daemon AFTER + /// the reset and re-pin the fans in manual. + /// + /// The barrier block itself drains: a submit can be overtaken between its + /// buffered append and its queued kick, and a bare sync would return + /// before that command ran. With submits already stopped by the caller, a + /// drain inside the barrier closes that window. + public func flush() { + queue.sync { [self] in drain() } + } } diff --git a/Tests/ThermalForgeTests/FanCommandPumpTests.swift b/Tests/ThermalForgeTests/FanCommandPumpTests.swift index b20ba59c..f7ccbb09 100644 --- a/Tests/ThermalForgeTests/FanCommandPumpTests.swift +++ b/Tests/ThermalForgeTests/FanCommandPumpTests.swift @@ -110,4 +110,37 @@ struct FanCommandPumpTests { #expect(box.value == expected) } } + + // MARK: - Flush barrier (quit ordering) + + /// The quit path's contract: resetAuto on a separate connection may only + /// land AFTER every already-submitted write. flush() returning must mean + /// exactly that — no polling, no timeout tolerance (the quit race was a + /// setRPM landing after the exit reset and re-pinning the fans in manual). + @Test("flush returns only after in-flight and pending writes have executed") + func flushIsABarrier() { + let h = Harness() + let pump = FanCommandPump(queue: DispatchQueue(label: "test.pump")) { cmd in h.execute(cmd) } + pump.submit(.setRPM(1000)) // blocks in-flight inside the harness + h.firstStarted.wait() + pump.submit(.setMax) // queued behind the blocked one + pump.submit(.setRPM(2000)) // queued (no coalesce: setMax between) + h.releaseFirst.signal() + pump.flush() + #expect(h.recorded == [.setRPM(1000), .setMax, .setRPM(2000)]) + } + + @Test("the pump still serves submits made after a flush") + func usableAfterFlush() { + let h = Harness() + let pump = FanCommandPump(queue: DispatchQueue(label: "test.pump")) { cmd in h.execute(cmd) } + pump.submit(.setMax) + h.firstStarted.wait() // the harness blocks the FIRST command + h.releaseFirst.signal() // ...so release it before the barrier + pump.flush() + #expect(h.recorded == [.setMax]) + pump.submit(.resetAuto) + #expect(wait(h, forCount: 2)) + #expect(h.recorded == [.setMax, .resetAuto]) + } }