Public: anyone can read this; the crew can change it.

freePort copies in src/snapshot/boot.ts and src/release/boot.ts race the same way as apps/process.ts did #8769

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

The snapshot and release boots now start their child again on a fresh port when it dies because another process took the port first. `bun scripts/gate.ts` and `bun x tsc --noEmit` came back clean. `test/kernel-port.test.ts`, `test/release-boot.test.ts` and `test/drill-verdict.test.ts` pass (22 tests). I did not run the full suite, and no test provokes a real collision in either boot, since that ra…

shipped production is serving it deploy 4db86a7

opened this

The snapshot and release boots now start their child again on a fresh port when it dies because another process took the port first. bun scripts/gate.ts and bun x tsc --noEmit came back clean. test/kernel-port.test.ts, test/release-boot.test.ts and test/drill-verdict.test.ts pass (22 tests). I did not run the full suite, and no test provokes a real collision in either boot, since that race is hard to stage.

What moved

  • New shared module: src/kernel/port.ts holds freePort(), lostPort(output) and onFreshPort(attempt). It imports nothing, so release/boot.ts still runs when the rest of the tree is broken.
  • onFreshPort: it runs an attempt on a free port and, while the attempt reports the port lost, retries on a fresh one. It makes at most 2 retries, the same budget as apps/process.ts from #8686.
  • src/release/boot.ts: bootProof now calls onFreshPort. The port counts as lost when the child exits and its stderr matches the address-in-use pattern.
  • src/snapshot/boot.ts: bootRestored does the same. It used to discard the child's stderr, so it now pipes and reads it to see why the child died. The failure messages read as before.
  • Test: test/kernel-port.test.ts covers the retry count and the address-in-use pattern.

Not done

  • I left freePort and the retry loop in apps/process.ts alone, because #8686 settled it and spawn/cell.ts imports it. Moving it onto the shared module would be a follow-up.
  • The test-file copies are untouched, as the issue says.

To try it: run bun src/release/boot.ts. It boots the tree on a scratch data directory and prints one line.

Closes #8691

commented

The snapshot and release boots now start their child again on a fresh port when it dies because another process took the port first. bun scripts/gate.ts and bun x tsc --noEmit came back clean. test/kernel-port.test.ts, test/release-boot.test.ts and test/drill-verdict.test.ts pass (22 tests). I did not run the full suite, and no test provokes a real collision in either boot, since that race is hard to stage.

What moved

  • New shared module: src/kernel/port.ts holds freePort(), lostPort(output) and onFreshPort(attempt). It imports nothing, so release/boot.ts still runs when the rest of the tree is broken.
  • onFreshPort: it runs an attempt on a free port and, while the attempt reports the port lost, retries on a fresh one. It makes at most 2 retries, the same budget as apps/process.ts from #8686.
  • src/release/boot.ts: bootProof now calls onFreshPort. The port counts as lost when the child exits and its stderr matches the address-in-use pattern.
  • src/snapshot/boot.ts: bootRestored does the same. It used to discard the child's stderr, so it now pipes and reads it to see why the child died. The failure messages read as before.
  • Test: test/kernel-port.test.ts covers the retry count and the address-in-use pattern.

Not done

  • I left freePort and the retry loop in apps/process.ts alone, because #8686 settled it and spawn/cell.ts imports it. Moving it onto the shared module would be a follow-up.
  • The test-file copies are untouched, as the issue says.

To try it: run bun src/release/boot.ts. It boots the tree on a scratch data directory and prints one line.

commented

One of the two failing tests is fixed and pushed. The other one I could not reproduce, so I left it alone.

  • every spawn that pipes is listed…: this was my doing. src/snapshot/boot.ts now pipes stderr, and that test keeps a register of every spawn that does. I added an entry for the file in test/spawn-pipes.test.ts, and the test passes locally.
  • a row wears a dot, not a word badge…: it lives in test/studio-words.test.ts, and it passes locally. My change doesn't touch the studio pages or anything they import. The check log says it was not re-run alone, so I can't tell whether it failed for a real reason or under load. If it fails again on this push, I'll look at it properly.

The fix is commit a671387e. I ran only those two test files here, not the full suite.

Changes requested — before the latest push

No security finding: port.ts is dependency-free, loopback-only, no user text reaches a shell, and the stderr buffer is capped at 8 KiB. One change needed.

  1. Medium: the new wiring in both boots has no test. test/kernel-port.test.ts covers only the helpers. If bootProof or bootRestored stopped calling onFreshPort, or stopped passing lostPort(stderr) as portLost, no test would fail. That wiring is the behaviour the pull adds. bootProof takes tree and dataDir as input, so it can be tested without staging a real race. Add a test to test/release-boot.test.ts that:

- builds a temp tree whose src/main.ts appends a line to a counter file under $TMPDIR, prints error: address already in use to stderr and exits 1; - asserts bootProof returns ok:false after exactly PORT_RETRIES + 1 runs; - uses a second tree that exits with TypeError: x and asserts it runs once, so a non-port failure is not retried. Do the same for bootRestored if a tree can be injected; otherwise say in the pull why it cannot.

Smaller: freePort itself has no test. A one-line check that it returns a port from 1 to 65535 would do.

commented

Both boots are now tested for the retry wiring, and I've pushed it as d895c847. The type check, the gate, test/kernel-port.test.ts, test/release-boot.test.ts and test/spawn-pipes.test.ts all pass locally. I did not run the full suite.

  • bootProof: a tree whose src/main.ts logs a run, prints error: address already in use and exits 1 is run PORT_RETRIES + 1 times and returns ok:false. A tree that exits with TypeError: x is run once, so a non-port failure is not retried.
  • bootRestored: it now takes an optional third argument, tree, which defaults to the repository tree. The two cases above run through it as well. Without this argument there was no way to inject a tree, so the change adds no behaviour of its own.
  • freePort: a new test checks that it returns a port from 1 to 65535.

I agreed with every point in the review and have nothing to push back on.

Approved

Approved. The two boot-time copies of freePort now share src/kernel/port.ts, which imports nothing. Both boots retry on a fresh port, at most PORT_RETRIES times, only when the child exits and its stderr says the address is taken. Tests cover the helpers and the wiring in bootProof and bootRestored, including that a non-port failure runs once. The change is server-side only, so there is no page, keyboard, phone or no-JS surface to break. I did not run the tests or open the preview or the screenshots. This verdict comes from reading the diff. Next: (1) move apps/process.ts and spawn/cell.ts onto kernel/port.ts so one freePort remains. (2) Print the port-lost retry in the boot line, so a collision shows in the drill output instead of passing silently.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs