fix(command-mode): drain process output concurrently to prevent TerminalService pipe deadlock#430
fix(command-mode): drain process output concurrently to prevent TerminalService pipe deadlock#430postoso wants to merge 1 commit into
Conversation
…nalService pipe deadlock
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3101ebd992
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| async let pendingOutput = Task.detached { outputHandle.readDataToEndOfFile() }.value | ||
| async let pendingError = Task.detached { errorHandle.readDataToEndOfFile() }.value |
There was a problem hiding this comment.
When Command Mode runs a command that produces unbounded or very large output, such as yes or a verbose build that runs until the 30s timeout, these detached reads now keep the pipes drained while accumulating the entire stream in Data before returning. That removes the previous OS-pipe-buffer cap, so the app can allocate hundreds of MB/GB and be killed before the timeout; please keep draining but impose a maximum captured byte count/truncation policy.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3101ebd992
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| async let pendingOutput = Task.detached { outputHandle.readDataToEndOfFile() }.value | ||
| async let pendingError = Task.detached { errorHandle.readDataToEndOfFile() }.value |
There was a problem hiding this comment.
When Command Mode runs a command that produces unbounded or very high-volume output, such as yes, tail -f, or dumping a large file, these detached readDataToEndOfFile() calls now keep draining and accumulating the entire stream in memory until the process exits or the 30s timeout kills it. Removing the pipe backpressure fixes the 64KB deadlock, but without a maximum retained byte count this can consume large amounts of app memory and then append/send an enormous JSON tool result; consider draining while retaining only a bounded prefix/suffix or terminating once a limit is exceeded.
Useful? React with 👍 / 👎.
1 participant
What
TerminalService.execute()(used by Command Mode to run shell commands) calledprocess.waitUntilExit()BEFORE draining stdout/stderr. A command that writes more than the ~64KB pipe buffer blocks onwrite()once the pipe fills, while the parent blocks inwaitUntilExit()— a classic pipe deadlock. The 30s timeout then firesterminate(), so high-output commands (seq,catof a large file,git log,ls -R) spuriously hang ~30s and return truncated output withsuccess=false.Repro of the old logic against
seq 1 200000: output truncated at exactly 65536 bytes,success=false, hung for the full timeout. After the fix: the complete 1288894 bytes,success=true, in ~68ms.How
Drain stdout and stderr concurrently (each on a detached read task) started before
waitUntilExit(), then await both and read termination status. Timeout behavior, return shape, and output trimming are unchanged.Tests
Two methods added to
DictationE2ETests(large stdout and large stderr): assert success, complete final line, output exceeding the 64KB buffer, exact line count, and prompt return (not the ~30s timeout).xcodebuild testgreen,swiftlint --strictclean.