Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions Sources/BestASRKit/Engines/ExternalProcessEngine.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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()
Expand All @@ -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
Expand Down
4 changes: 3 additions & 1 deletion Tests/BestASRKitTests/ExternalEngineTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading