mirror of
https://github.com/AgentSeal/codeburn.git
synced 2026-07-20 14:34:50 +00:00
fix(menubar): run CLI exit-wait and timeout off the cooperative pool
The menubar wedged on "Loading Today…" for hours after an idle period. Root cause: DataClient.runCLI called the blocking process.waitUntilExit() from an async function on Swift's cooperative thread pool. On a 16-core machine, 16 concurrent slow `codeburn` subprocesses pinned all 16 cooperative threads inside waitUntilExit; the 45s timeout — itself a Task on that same pool — could then never be scheduled to kill them, so the deadlock was permanent. Confirmed via sample: 16/16 cooperative threads parked in waitUntilExit. PR #412 (AppStore inFlightKeys bookkeeping) was a layer above the OS-thread deadlock and could not fix it. Move both blocking points off the cooperative pool: bridge waitUntilExit through a global (overcommit) queue via a continuation, and drive the timeout from a DispatchSource on a global queue so it fires even when the pool is saturated. Extract runProcess for testability; add a concurrency + timeout smoke test and an output/exit-code test.
This commit is contained in:
parent
69b1736365
commit
4ffec37861
2 changed files with 83 additions and 6 deletions
|
|
@ -49,7 +49,7 @@ struct DataClient {
|
|||
}
|
||||
}
|
||||
|
||||
private struct ProcessResult {
|
||||
struct ProcessResult {
|
||||
let stdout: Data
|
||||
let stderr: String
|
||||
let exitCode: Int32
|
||||
|
|
@ -57,7 +57,26 @@ struct DataClient {
|
|||
|
||||
private static func runCLI(subcommand: [String]) async throws -> ProcessResult {
|
||||
let process = CodeburnCLI.makeProcess(subcommand: subcommand)
|
||||
return try await runProcess(process,
|
||||
timeoutSeconds: spawnTimeoutSeconds,
|
||||
label: subcommand.joined(separator: " "))
|
||||
}
|
||||
|
||||
/// Runs an already-configured process to completion, draining its output and
|
||||
/// enforcing a hard timeout.
|
||||
///
|
||||
/// CRITICAL: neither the timeout nor the exit wait may run on Swift's
|
||||
/// cooperative thread pool. `process.waitUntilExit()` is a blocking syscall;
|
||||
/// on a 16-core machine, 16 concurrent slow CLIs would pin all 16 cooperative
|
||||
/// threads inside waitUntilExit, exhausting the pool. A timeout living on that
|
||||
/// same pool could then never be scheduled to kill the hung processes — the
|
||||
/// menubar deadlocks on "Loading…" forever (confirmed via sample: 16/16
|
||||
/// cooperative threads parked in waitUntilExit). So the timeout is a
|
||||
/// DispatchSource on a global queue, and the exit wait is bridged through a
|
||||
/// global (overcommit) queue instead of blocking the caller's executor.
|
||||
static func runProcess(_ process: Process,
|
||||
timeoutSeconds: UInt64,
|
||||
label: String) async throws -> ProcessResult {
|
||||
let outPipe = Pipe()
|
||||
let errPipe = Pipe()
|
||||
process.standardOutput = outPipe
|
||||
|
|
@ -69,15 +88,17 @@ struct DataClient {
|
|||
throw DataClientError.spawn(error.localizedDescription)
|
||||
}
|
||||
|
||||
let timeoutTask = Task.detached(priority: .utility) {
|
||||
try? await Task.sleep(nanoseconds: spawnTimeoutSeconds * 1_000_000_000)
|
||||
let timeoutTimer = DispatchSource.makeTimerSource(queue: DispatchQueue.global(qos: .utility))
|
||||
timeoutTimer.schedule(deadline: .now() + .seconds(Int(timeoutSeconds)))
|
||||
timeoutTimer.setEventHandler {
|
||||
if process.isRunning {
|
||||
NSLog("CodeBurn: CLI subprocess timed out after %llus for %@ — terminating",
|
||||
spawnTimeoutSeconds, subcommand.joined(separator: " "))
|
||||
timeoutSeconds, label)
|
||||
terminateWithEscalation(process)
|
||||
}
|
||||
}
|
||||
defer { timeoutTask.cancel() }
|
||||
timeoutTimer.resume()
|
||||
defer { timeoutTimer.cancel() }
|
||||
|
||||
let outHandle = outPipe.fileHandleForReading
|
||||
let errHandle = errPipe.fileHandleForReading
|
||||
|
|
@ -90,7 +111,12 @@ struct DataClient {
|
|||
}
|
||||
try? outHandle.close()
|
||||
try? errHandle.close()
|
||||
process.waitUntilExit()
|
||||
await withCheckedContinuation { (continuation: CheckedContinuation<Void, Never>) in
|
||||
DispatchQueue.global(qos: .utility).async {
|
||||
process.waitUntilExit()
|
||||
continuation.resume()
|
||||
}
|
||||
}
|
||||
|
||||
if out.count >= maxPayloadBytes {
|
||||
throw DataClientError.outputTooLarge
|
||||
|
|
|
|||
51
mac/Tests/CodeBurnMenubarTests/DataClientProcessTests.swift
Normal file
51
mac/Tests/CodeBurnMenubarTests/DataClientProcessTests.swift
Normal file
|
|
@ -0,0 +1,51 @@
|
|||
import XCTest
|
||||
@testable import CodeBurnMenubar
|
||||
|
||||
final class DataClientProcessTests: XCTestCase {
|
||||
/// Concurrency + timeout smoke test: launch more hung subprocesses than
|
||||
/// there are cooperative threads, all at once, with a short timeout, and
|
||||
/// assert every call returns once the timeout kills its sleep.
|
||||
///
|
||||
/// NOTE: this does NOT reproduce the production permanent deadlock (16/16
|
||||
/// cooperative threads parked in waitUntilExit). In a short-lived unit-test
|
||||
/// process libdispatch spins up replacement threads for blocked workers, so
|
||||
/// even the old blocking-on-the-pool code completes here. The real deadlock
|
||||
/// built up over ~2 days under the @MainActor refresh loop and is confirmed
|
||||
/// by the live `sample`, not by this test. Kept as a guard that the
|
||||
/// off-pool wait + timeout path stays correct under concurrency.
|
||||
func testConcurrentTimedOutProcessesAllComplete() {
|
||||
let count = ProcessInfo.processInfo.activeProcessorCount * 2 + 4
|
||||
let done = DispatchSemaphore(value: 0)
|
||||
|
||||
Task {
|
||||
await withTaskGroup(of: Void.self) { group in
|
||||
for _ in 0..<count {
|
||||
group.addTask {
|
||||
let process = Process()
|
||||
process.executableURL = URL(fileURLWithPath: "/bin/sleep")
|
||||
process.arguments = ["30"]
|
||||
_ = try? await DataClient.runProcess(process, timeoutSeconds: 1, label: "sleep 30")
|
||||
}
|
||||
}
|
||||
}
|
||||
done.signal()
|
||||
}
|
||||
|
||||
// Wait on the XCTest thread (a real thread, not the cooperative pool) so
|
||||
// the deadlock is detectable even when the pool is fully starved.
|
||||
let outcome = done.wait(timeout: .now() + 15)
|
||||
XCTAssertEqual(outcome, .success,
|
||||
"runProcess deadlocked: \(count) concurrent CLIs starved the cooperative pool")
|
||||
}
|
||||
|
||||
/// A normally-exiting process returns its real output and exit code through
|
||||
/// the off-pool wait path.
|
||||
func testProcessReturnsOutputAndExitCode() async throws {
|
||||
let process = Process()
|
||||
process.executableURL = URL(fileURLWithPath: "/bin/echo")
|
||||
process.arguments = ["hello"]
|
||||
let result = try await DataClient.runProcess(process, timeoutSeconds: 5, label: "echo hello")
|
||||
XCTAssertEqual(result.exitCode, 0)
|
||||
XCTAssertEqual(String(data: result.stdout, encoding: .utf8), "hello\n")
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue