Skip to content

Fix command log streaming race#5789

Open
stefanhaller wants to merge 2 commits into
masterfrom
command-log-streaming-race
Open

Fix command log streaming race#5789
stefanhaller wants to merge 2 commits into
masterfrom
command-log-streaming-race

Conversation

@stefanhaller

Copy link
Copy Markdown
Collaborator

Serialize concurrent writes to the streamed command's output writer. runAndStreamAux funnels a command's stdout and stderr into a single cmdWriter (the command-log panel, or a buffer when output is suppressed) from two separate goroutines: stderr through the MultiWriter set on cmd.Stderr, and stdout through the onRun callback. Those goroutines
wrote the shared writer without any synchronization, racing on the prefixWriter's prefixWritten flag and interleaving the two streams. Wrap the writer so its writes are serialized.

stefanhaller and others added 2 commits July 9, 2026 15:12
runAndStreamAux funnels a command's stdout and stderr into a single
cmdWriter (the command-log panel, or a buffer when output is suppressed)
from two separate goroutines: stderr through the MultiWriter set on
cmd.Stderr, and stdout through the onRun callback. Those goroutines
wrote the shared writer without any synchronization, racing on the
prefixWriter's prefixWritten flag and interleaving the two streams.
Wrap the writer so its writes are serialized.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…uffers

runAndStreamAux reads the stdout buffer (and, when output is suppressed,
the combinedOutput buffer) for its error message after handler.wait()
returns, but the goroutine that fills those buffers by draining the
command's output isn't awaited, so the reads raced its final writes.

Own the goroutine here rather than letting the onRun callbacks spawn it,
and join it before reading the buffers. The pty reader reaches EOF on its
own once the process exits, but the non-pty pipe never does, so its
handler now closes the read end to unblock the reader; the pipe is
synchronous, so by the time the command has exited all of its output has
already been read and nothing is lost. This also plugs the goroutine that
the non-pty streaming path previously leaked on every command.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@stefanhaller stefanhaller added the maintenance For refactorings, CI changes, tests, version bumping, etc label Jul 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance For refactorings, CI changes, tests, version bumping, etc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant