From cae12d843c37b8d9f95b6f242192779108b348dd Mon Sep 17 00:00:00 2001
From: che cheng <kiki830621@gmail.com>
Date: Fri, 10 Jul 2026 08:36:58 +0800
Subject: [PATCH] =?UTF-8?q?fix:=20watchdog=20exits=20only=20via=20exit=20l?=
 =?UTF-8?q?atch=20or=20deadline=20kill=20=E2=80=94=20never=20a=20racy=20is?=
 =?UTF-8?q?Running=20read=20(#91)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The CI hang signature (test ran the stub's full 3600 s, error message
lacked 'timeout') proves the kill branch never executed: Process.isRunning
spuriously reported false right after run() on the starved runner, the
'while isRunning' watchdog exited on the spot, and waitUntilExit() blocked
unbounded. A terminationHandler-set thread-safe latch (installed before
run()) now gates the loop, so it can exit only when the adapter is really
gone or after SIGTERM/SIGKILL; waitUntilExit is bounded on both paths.
The hung-adapter stub drops to sleep 15 so any future regression costs
seconds of CI, not an hour.

Refs #91
---
 .../Engines/ExternalProcessEngine.swift       | 22 +++++++++++++++++--
 .../BestASRKitTests/ExternalEngineTests.swift |  4 +++-
 2 files changed, 23 insertions(+), 3 deletions(-)

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 {