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]) + } }