diff --git a/Sources/BestASRKit/Engines/ExternalProcessEngine.swift b/Sources/BestASRKit/Engines/ExternalProcessEngine.swift index 22d435f..db7051b 100644 --- a/Sources/BestASRKit/Engines/ExternalProcessEngine.swift +++ b/Sources/BestASRKit/Engines/ExternalProcessEngine.swift @@ -128,6 +128,18 @@ public struct ExternalProcessEngine: Engine { return sane.isEmpty ? fullText() : sane } + /// Thread-safe "the adapter really exited" latch, flipped by the + /// terminationHandler (fires on a private queue). The watchdog loops on + /// this instead of `Process.isRunning`, whose spurious `false` right after + /// `run()` under load skipped the watchdog entirely and left an unbounded + /// `waitUntilExit()` — the 1-hour CI hang of #91. + private final class ExitLatch: @unchecked Sendable { + private let lock = NSLock() + private var exited = false + var isSet: Bool { lock.withLock { exited } } + func set() { lock.withLock { exited = true } } + } + /// Spawn + collect with a hard timeout (D3): SIGTERM on expiry, SIGKILL /// if the process lingers, then the pipe read ends are closed so drain /// tasks unblock even if a grandchild still holds the write end. @@ -141,6 +153,9 @@ public struct ExternalProcessEngine: Engine { let errPipe = Pipe() process.standardOutput = outPipe process.standardError = errPipe + // Installed BEFORE run() so an instant exit can never miss the latch. + let exited = ExitLatch() + process.terminationHandler = { _ in exited.set() } do { try process.run() @@ -162,12 +177,15 @@ public struct ExternalProcessEngine: Engine { let deadline = Date().addingTimeInterval(timeout) var timedOut = false - while process.isRunning { + // Invariant (#91): this loop exits ONLY via the exit latch (the adapter + // is really gone) or the deadline kill — never via a racy liveness read, + // so waitUntilExit() below is bounded on both paths. + while !exited.isSet { if Date() > deadline { timedOut = true process.terminate() try? await Task.sleep(nanoseconds: 500_000_000) - if process.isRunning { + if !exited.isSet { kill(process.processIdentifier, SIGKILL) } // A grandchild may still hold the pipe write end — closing diff --git a/Tests/BestASRKitTests/ExternalEngineTests.swift b/Tests/BestASRKitTests/ExternalEngineTests.swift index 9e4df09..908b56a 100644 --- a/Tests/BestASRKitTests/ExternalEngineTests.swift +++ b/Tests/BestASRKitTests/ExternalEngineTests.swift @@ -113,7 +113,9 @@ struct ExternalEngineTests { @Test func `A hung adapter is terminated at the timeout`() async throws { let dir = try makeTempDir() defer { try? FileManager.default.removeItem(at: dir) } - let adapter = try makeAdapter(in: dir, script: "sleep 3600") + // 15 s: still 15x the 1 s test timeout, but a watchdog regression now + // costs seconds of CI, not the hour-long hang of #91 (sleep 3600). + let adapter = try makeAdapter(in: dir, script: "sleep 15") let hung = ExternalProcessEngine( id: .mlxAudio, command: [adapter], timeoutOverride: 1.0) do {