Repository navigation
fix: a ticket is written by several processes, so each writes a temp of its own - #979
Merged
Merged
Conversation
This was referenced Nov 8, 2026
kid7st
added a commit
that referenced
this pull request
Nov 8, 2026
…980) Root causes of the ticket tests' intermittent failures, and the fixes. It also corrects #979, whose explanation of the failure that stopped v0.30.0's first publish was wrong. ## Three causes ### 1. A test's fake step rewrote the ticket in place (the publish failure, and `ticket.test.ts` "a newer …" on Node 26) `SyntaxError: Unexpected non-whitespace character after JSON at position 215`, in `readTicket` as `startTicket` polled. The fake step in `waitingStep` wrote the ticket with `writeFileSync(path, …)`, which truncates and rewrites the same file, while the starting command read it. `readFileSync(path, "utf8")` reads until EOF in several reads, so it can return the first bytes of the old ticket and then the tail of the new one: a whole JSON document followed by more. The ticket that test creates is exactly 215 bytes; the earlier failure says 213. Reproduced: a process rewriting a file in place, short then long, and another reading it, gives exactly this error, always at the short document's length (14 to 33 times in about 110,000 reads). With write-then-rename, 290,000 reads give none. The product never writes a ticket in place (`writeFileAtomic`), so this was the test. Its two fake steps now write the ticket whole, as a step's process does. **#979 said the cause was two `writeFileAtomic` writers sharing a temp. That race is real, but it is not what happened here**: this ticket had no second `writeFileAtomic` writer. ### 2. A stop's end lost to the end the stopped step wrote itself (`cli-deploy.test.ts` "hands a signed-out host CLI's sign-in", twice on Node 26) `stopTicket` signalled the step's process group and then wrote `stopped on request`. A step's process does not die at once on SIGTERM: `proper-lockfile` loads `signal-exit`, which catches SIGTERM in JavaScript and re-raises it. On Linux, the step's event loop can deliver the exit of its host CLI (killed by the same signal) before that handler. The step then records its own failure (`` `railway login --browserless` did not sign in (exit 1) ``; `exitOf` maps a death by signal to 1), and `update` keeps the first end, so the stop's was dropped. A person who stopped a deploy was told the host CLI had failed to sign in. Measured in a Linux container (Node 26.13.0, 4 CPUs): a process loading `signal-exit`, its child in its group, the group sent SIGTERM; the step ran its child-exit handler before dying 3 times in 300, 13 in 300 under load. Without `signal-exit`, 0 in 600. On macOS, 0 in 500 either way. Superseding a step (`replaced by a newer attempt`) and giving up on a start had the same order. ### 3. `update` read, changed and wrote the ticket with nothing between (what #2 needs to be fixed for good) Writing the end first is not enough on its own: a step that read its ticket as running just before another process ended it would write its record after, and reopen it. ## The fix A ticket is two files: the step's own record, written by one process at a time (the starting command, then the step), and its **end**, created once with a hard link, which the kernel lets only one writer make. `readTicket` reads the end over the record. An end is written before what it decides is done: `stopTicket`, `supersede` and a start given up on now end the ticket, then stop the process. So the stop is the end even when the step hears the consequences first, nothing the step writes after reopens it, and two enders that both read the ticket as running end it once. `prune` removes every file of a ticket past its time, a crashed writer's draft included. `writeTicket` no longer needs `sharedWriters` (#979): the record has one writer at a time. The `.address` keeps it, since two `--check --code` may hand one over at once. ## Tests - A step run by the real `runTicket` that records its own failure when told SIGTERM (what a step with `signal-exit` does when it hears its child die first): a stop on request is its end; a newer attempt is the end of the one it replaces; a start given up on is its end. - A step's own late write after another process ended its ticket does not reopen it. - `endTicket`: a second end, by one that read the ticket running too, leaves the first. All four of the first group failed before the fix, for the reasons above (the step's own message won; the ticket was reopened to `need_user_action`). Mutation checks, each failing a test: the process stopped before the end is written (stop, supersede), a start given up on not stopping its process, the end not read over the record, the end written by rename instead of a link. The ticket tests and `cli-deploy`/`cli-login` also passed three times under load in the Linux container. ## Found, not fixed here `running(pid)` is `kill(pid, 0)`, which succeeds for a zombie. In a container whose PID 1 does not reap (Docker without `--init`), a stopped step stays a zombie, so `stop` waits its full 5 s and `--check` never sees the step gone. CI's runner reaps, so it does not fail there. A separate change.
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.
publish.ymlfor v0.30.0 failed its Test step, so nothing was published:test/ticket.test.ts("a step still getting ready … is replaced: a newer attempt is never locked out") died inreadTicketwithSyntaxError: Unexpected non-whitespace character after JSON at position 215. The same test family had failed intermittently on Node 26 in earlier CI runs (ticket.test.ts"a newer …",cli-deploy.test.ts"hands a signed-out host CLI's sign-in…"), which I had filed as flaky. It is a race in the code.The race
writeFileAtomicwrites to a fixed temp,<path>.tmp, and renames it. Its own comment says that rests on one process writing a state root. A ticket is written by several at once: the command that started the step, the step's own process, every--check, and a newer step that replaces an older one. Two of them on one ticket either rename each other's temp away (ENOENTonrename) or write one temp at once, and the interleaved bytes are whatreadTicketthen fails to parse.Reproduced: two processes calling
writeFileAtomicon one path for 400 rounds each both crash withENOENT rename.The fix
writeTicketand the.addresswrite passsharedWriters, which gives each writer a temp of its own (the form the model catalog already uses for the same reason); the last rename wins. The header comment ofticket.ts, which said a ticket was written by one process at a time, is corrected.What this does not claim
ticket.test.tsunder 12 busy loops, and 20 without, passed before and after).update()atomic. It reads, changes and writes the ticket, so two writers can still lose each other's change (the last writer wins), where before they could corrupt the file. Whether that matters is a separate question: a ticket that has ended is not reopened (updatereturns it as it ended), which is the case the tests exercise.Tests
atomic-write.test.ts: two real processes, 300 writes each, to one path. With shared writers both finish and the file is whole; without, one fails (this is the race, held as the function's contract).ticket-writers.test.ts: the ticket file and the address file are each written through a temp of their own.Mutation checks, each failing a test: the ticket file and, separately, the address file back to the fixed temp (the second survived at first, because the address is written only to a step waiting on its person, so the test now puts the ticket in that state), and
sharedWritersignored inwriteFileAtomic.Correction (after merge)
The explanation above is wrong for the failure it names. The ticket in
ticket.test.tsthat failed had no secondwriteFileAtomicwriter: a test's fake step rewrote it in place withwriteFileSync, andreadFileSyncreturned the old ticket's first bytes and the new one's tail. Root cause, measurement and fix: #980. The race this PR guards against (two writers on one fixed temp) is real for the.addressfile, which keepssharedWriters; #980 gives the ticket's record one writer at a time and its end a file created once.