Skip to content

Commit fbfa656

Browse files
grokifyclaude
andcommitted
fix(desktop): drain command output concurrently to avoid Process deadlock
ProcessCommandExecutor called waitUntilExit() before reading either pipe. Once combined stdout+stderr exceeds the ~64KB pipe buffer, the child blocks on write() with nothing draining it, so it never exits and the continuation never resumes. TerminalWrapperDetector's `ps -Ao pid=,command=` call is the first caller whose output realistically exceeds that buffer (174KB on a typical dev machine with the browser and other apps running), so this deadlocked SessionManager.refresh() on every launch — the app never left its "Loading..." state. Read both pipes on background queues while the process runs, then wait for exit. Verified the regression reproduces without the fix: swift test itself hung past the 2-minute harness timeout and had to be force-killed, not just the individual test case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent bb7e48f commit fbfa656

2 files changed

Lines changed: 67 additions & 3 deletions

File tree

apps/desktop/Sources/PlexusOneDesktop/Services/CommandExecuting.swift

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,27 @@ struct ProcessCommandExecutor: CommandExecuting {
2020

2121
do {
2222
try process.run()
23-
process.waitUntilExit()
2423

25-
let stdoutData = stdoutPipe.fileHandleForReading.readDataToEndOfFile()
26-
let stderrData = stderrPipe.fileHandleForReading.readDataToEndOfFile()
24+
// Drain both pipes concurrently with the process running. Reading
25+
// only after waitUntilExit() deadlocks once combined output
26+
// exceeds the pipe buffer (~64KB): the child blocks on write()
27+
// with nothing reading, so it never exits.
28+
var stdoutData = Data()
29+
var stderrData = Data()
30+
let readGroup = DispatchGroup()
31+
32+
readGroup.enter()
33+
DispatchQueue.global(qos: .utility).async {
34+
stdoutData = stdoutPipe.fileHandleForReading.readDataToEndOfFile()
35+
readGroup.leave()
36+
}
37+
readGroup.enter()
38+
DispatchQueue.global(qos: .utility).async {
39+
stderrData = stderrPipe.fileHandleForReading.readDataToEndOfFile()
40+
readGroup.leave()
41+
}
42+
readGroup.wait()
43+
process.waitUntilExit()
2744

2845
let result = CommandResult(
2946
exitCode: process.terminationStatus,
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import XCTest
2+
@testable import PlexusOneDesktop
3+
4+
final class CommandExecutingTests: XCTestCase {
5+
6+
/// `ps -Ao pid=,command=` on a typical dev machine easily exceeds the ~64KB
7+
/// pipe buffer. Reading stdout only after waitUntilExit() deadlocks in that
8+
/// case: the child blocks on write() with nothing draining the pipe, so it
9+
/// never exits and the continuation never resumes.
10+
func testExecuteHandlesOutputLargerThanPipeBuffer() async throws {
11+
let executor = ProcessCommandExecutor()
12+
13+
// Print well over 64KB to stdout, interleaved with some stderr output,
14+
// to exercise both pipes concurrently.
15+
let script = """
16+
for i in $(seq 1 20000); do echo "line $i of padding output to exceed the pipe buffer"; done
17+
echo "some stderr output" >&2
18+
"""
19+
20+
let result = try await withTimeout(seconds: 10) {
21+
try await executor.execute("/bin/sh", arguments: ["-c", script])
22+
}
23+
24+
XCTAssertEqual(result.exitCode, 0)
25+
XCTAssertGreaterThan(result.stdout.utf8.count, 64 * 1024)
26+
XCTAssertTrue(result.stdout.contains("line 20000 of padding"))
27+
XCTAssertTrue(result.stderr.contains("some stderr output"))
28+
}
29+
30+
private func withTimeout<T: Sendable>(
31+
seconds: TimeInterval,
32+
_ operation: @escaping @Sendable () async throws -> T
33+
) async throws -> T {
34+
try await withThrowingTaskGroup(of: T.self) { group in
35+
group.addTask { try await operation() }
36+
group.addTask {
37+
try await Task.sleep(nanoseconds: UInt64(seconds * 1_000_000_000))
38+
throw TimeoutError()
39+
}
40+
let result = try await group.next()!
41+
group.cancelAll()
42+
return result
43+
}
44+
}
45+
46+
private struct TimeoutError: Error {}
47+
}

0 commit comments

Comments
 (0)