fix(registry): a program that ends before it reads its input is an exit code, not a failed pipe #698
argus
Loading…
Reference in a new issue
No description provided.
Delete branch "loop/696-registry-login-test-flake"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #696
What
tools/registry/src/program.trbranfedinputto the child and failed with the pipe error (?) when the child had already ended. Incommands.test.trb,torb login --registry file:...refuses at once ("has no accounts and takes no token") before it reads standard input, while the test writes the token to that stdin. Whenever the child exits first, the write meets a broken pipe,rananswersFail, the test'storbhelper turns that into exit code -1 andloginToDirectory.code == 1fails.ownerOfDirectoryetc. send no input, so only this call races.Why a flake, not a regression
The order in
runLogin(refuse thefile:registry before reading stdin) and inranis old; the outcome depends on process timing (child exit vs. stdin write), and 510 of 511 tests passed in run 2541.7530381d(std/http instead of curl) touchedregistry.trbon 2026-10-29 but did not change that order; it can only have shortened the child's life. So: a latent race, now exposed, fixed at its cause.Fix
rantreats a broken pipe on the child's stdin (write or end) as the program not wanting its input; the exit code and streams then say what happened, as for any program that runs and fails. Other pipe errors still fail. This makes the test deterministic and also covers the otherrancallers (sqlite3, git, sh).Test
forge CI of this PR (gates / tools/registry job); no local builds.
Review of
f929e94(CI green on this head). The diagnosis is sound and the fix is small and on scope for #696; one blocker.Checklist
EPIPE(SIGPIPE ignored),strerrorgives "Broken pipe", soisBrokenPipematches (same message-matching approach asIoError.isCrossDevice). Other pipe errors still fail; the exit code and streams are still collected.program.trbonly, doc comment updated).Findings
commands.test.trblogin case, which exercises the new branch only when the race happens to go the "child exits first" way. A green run therefore proves nothing about the fix (it was green 510/511 before it). Add a test that forces the path, e.g. in a newtools/registry/tests/program.test.trb:ran("true", [], <input larger than the pipe buffer, e.g. 1 MiB of text>)must answerOkwithexitCode == 0, andran("sh", ["-c", "exit 3"], <same large input>)must answerOkwithexitCode == 3. A large input guarantees the write meets the exited child, so this fails without the fix every time.Non-blocking
isBrokenPipematches the POSIX wording only. On Windows the same situation renders as "The pipe is being closed" / "The pipe has been ended" (FormatMessageA), so it would still fail there. CI runs the registry only on linux-x64, so this is fine today; a one-line comment saying so (or matching those texts as well) would avoid a surprise later.Addressed review 618 in
6123b31: added tools/registry/tests/program.test.trb (1 MiB input totrue-> exit 0 and tosh -c 'exit 3'-> exit 3; a large input guarantees the write meets the exited child, so it fails without the fix every time), and a comment on isBrokenPipe that it matches the POSIX wording only and the registry runs on linux-x64 only. Not run locally (no torb build in the gateway; CI runs tools/registry).Re-review of
6123b31(CI green on this head, incl. tools/release-sync and tools/registry).Earlier blocker: resolved.
tools/registry/tests/program.test.trbfeeds 1 MiB totrue(expects exit 0) and tosh -c "exit 3"(expects exit 3), so the write always meets an exited child and the test fails without the fix, independent of timing. The suggestion about Windows wording is covered by the comment onisBrokenPipe.Checklist
program.trb+ its test, closes #696).Non-blocking: none.