Skip to content

Fix output loss in quiet mode when a task fails - #22

Merged
kozak merged 1 commit into
mainfrom
fix-quiet-mode-output-race
Aug 6, 2026
Merged

Fix output loss in quiet mode when a task fails#22
kozak merged 1 commit into
mainfrom
fix-quiet-mode-output-race

Conversation

@kozak

@kozak kozak commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

The quiet-mode-failure golden test has been flaky, and it is a real bug rather than
test noise. It blocked the v0.18.0.13 release: the arm64 job failed on it, matrix
fail-fast cancelled x86_64, and no release was published.

The bug

App.hs flushed the quiet buffer as soon as the child exited, but the stdout/stderr
handlers — which are what fill that buffer (Utils.hs:46) — were only awaited
afterwards:

exitCode <- waitForProcess processHandle
when quietMode $ ... flushQuietBuffer ...   -- flushed here
timeoutStream ... $ wait stdoutHandler      -- drained here

A child exiting does not mean its pipes are drained. Under load the flush emitted
nothing and the real output was appended to a buffer nobody read again — hence the
observed symptom of both lines vanishing, leaving only -- output: and
-- exit code: 1. Fix: drain the handlers first.

Separately, both handlers append concurrently via plain modifyIORef, which can lose
one of two racing updates. Now atomicModifyIORef'.

Test

quiet-mode-failure-large-output fails 8/8 without the fix and passes 8/8 with it.
The existing single-line test only failed about half the time locally, which is why this
survived so long.

Detection needs enough output that draining it takes measurable time after exit — and
that exposed the deadlock already flagged by a FIXME in the harness. LBS.hGetContents
is lazy, so nothing was read until after waitForProcess, and any task writing more than
the pipe buffer holds (~64 KiB) blocked forever. Forcing the read before reaping is
sufficient: std_out/std_err are passed as UseHandle, so the process library closes
the parent's copy of the write end and the read terminates at EOF.

Full suite passes.

Note

The release workflow's matrix fail-fast is worth turning off regardless — one flaky
arm64 job should not cancel the x86_64 build and leave a tag with no release asset.

🤖 Generated with Claude Code

The quiet buffer was flushed as soon as the child exited, but the stdout and stderr
handlers that fill it were only awaited afterwards. A child exiting does not mean its
pipes have been drained, so on a loaded machine the flush emitted nothing and the real
output was appended to a buffer nobody read again. Drain the handlers first.

This is what made the quiet-mode-failure golden test flaky - it dropped both expected
lines and left only "-- output:" and "-- exit code: 1". It blocked the v0.18.0.13
release: the arm64 job failed on it and matrix fail-fast cancelled x86_64, so no
release was published.

Also make the append atomic. Both handlers write to the buffer concurrently, and plain
modifyIORef can lose one of two racing updates.

The new test fails 8/8 without the fix and passes 8/8 with it, where the existing
single-line test only failed about half the time locally. Detection needs enough lines
that draining them takes measurable time after exit, which in turn exposed the deadlock
already flagged by a FIXME in the harness: hGetContents is lazy, so nothing was read
until after waitForProcess and a task writing more than the pipe buffer holds (~64 KiB)
blocked forever. Forcing the read first is enough, since UseHandle closes the parent's
copy of the write end and the read ends at EOF.
@kozak
kozak requested a review from zyla August 5, 2026 21:37
@kozak
kozak merged commit 3b77557 into main Aug 6, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants