Repository navigation
sed_test: the input is in the pipe before sed runs - #1055
Merged
Merged
Conversation
runsed() spawned sed on a pipe and then wrote its input. For a script
sed refuses before it reads (an unclosed s/, a lone "{" or "}", ":" with
no label) sed can end and close its end of the pipe first; devpipe's
pipewrite then kills the writer, the test itself, with "write on closed
pipe", and run() counts that as a failure with no message. CI's Linux
AMD64 Build & Test showed exactly that: "--- FAIL: ShortScripts" and the
address line, nothing else, on a PR that does not touch sed.
Locally it passed 16/16 under -c1 and -c0; with a 50 ms sleep before the
write ShortScripts and Unclosed failed every time with the same bare
FAIL. The input is now written while the test still holds the read end,
so nothing sed does can close the pipe under the write; with the same
50 ms window kept after the spawn, it passes under both.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0195u6z6W3TbdAx7jg6RqhYy
pdfinn
pushed a commit
that referenced
this pull request
Nov 3, 2026
Brings in #1055, the fix for the sed_test race that was this PR's one red job (Linux AMD64 Build & Test, ShortScripts). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195u6z6W3TbdAx7jg6RqhYy
10 of 11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
sed_test'sShortScriptsfailed in Linux AMD64 Build & Test on #1054, which does not touch sed. The log has only--- FAIL: ShortScriptsand the address line, with no assertion message. The cause is a race in the test.runsed()spawned sed on a pipe and only then wroteabc\ninto it.s/, a lone{or},:with no label. For those, sed can exit and close the read end before step 1's write.emu/port/devpipe.c'spipewritethen kills the writer, which is the test itself, withwrite on closed pipe.run()catches that as a failure with no message.Unclosedhas the same exposure.Changes
tests/sed_test.b,runsed(): the input is written, and the write end dropped, before sed is spawned. The test still holds the read end at that point, and the pipe buffers the 4 bytes, so sed can't close the pipe under the write. A short write is reported ascrash: cannot write inputrather than passing silently.Testing
Linux AMD64, headless build.
-c1and-c0, 8 each). It is timing-dependent, which is why it rarely shows.ShortScriptsandUnclosedfail on every run, with the same bare FAIL as CI.-c1and-c0. It still passes with a 50 ms sleep left in after the spawn./tests/runner.dis -v: not run in full locally; this PR's CI runs it.Checklist
.disbuild artifacts committed fromappl/ortests/Generated by Claude Code