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

mason/forge:mason/8691-freeport-copies-in-src-snapshot-boot-ts → main 3 commits fork

  • Merged. mason/8691-freeport-copies-in-src-snapshot-boot-ts was merged into main as fa10b71.

Review

Sign in to review this change.
6 files changed +142 −29
src/kernel/port.ts added +31
@@ -0,0 +1,31 @@
1 /**
2 * A loopback port for a child that binds it later, and the retry for when
3 * another process takes it in between. Imports nothing: `release/boot.ts`
4 * must run when the rest of the tree is what is broken.
5 */
6
7 /** How many more times a child that lost its port is started on a fresh one. */
8 export const PORT_RETRIES = 2;
9 const ADDRESS_IN_USE_RE = /address (?:already )?in use|EADDRINUSE|port \d+ in use/i;
10
11 /** A port the OS says is free right now. */
12 export async function freePort(): Promise<number> {
13 const probe = Bun.serve({ hostname: "127.0.0.1", port: 0, fetch: () => new Response("") });
14 const port = probe.port ?? 0;
15 await probe.stop(true);
16 return port;
17 }
18
19 /** Whether a child's output says it could not bind its port. */
20 export function lostPort(output: string): boolean {
21 return ADDRESS_IN_USE_RE.test(output);
22 }
23
24 /** Run `attempt` on a free port; while it reports the port lost, on a fresh one, up to PORT_RETRIES more times. */
25 export async function onFreshPort<T>(
26 attempt: (port: number) => Promise<{ readonly result: T; readonly portLost: boolean }>,
27 ): Promise<T> {
28 let run = await attempt(await freePort());
29 for (let retry = 0; retry < PORT_RETRIES && run.portLost; retry++) run = await attempt(await freePort());
30 return run.result;
31 }
src/release/boot.ts +20 −13
@@ -28,6 +28,7 @@ import { tmpdir } from "node:os";
2828 import { join } from "node:path";
2929 import { running } from "../kernel/version.ts";
3030 import { probeHeaders } from "../kernel/probe.ts";
31 import { lostPort, onFreshPort } from "../kernel/port.ts";
3132
3233 export const BOOT_BUDGET_MS = 120_000;
3334 const POLL_MS = 250;
@@ -49,13 +50,6 @@ export interface BootInput {
4950 readonly env?: Readonly<Record<string, string | undefined>>;
5051 }
5152
52 async function freePort(): Promise<number> {
53 const probe = Bun.serve({ hostname: "127.0.0.1", port: 0, fetch: () => new Response("") });
54 const port = probe.port ?? 0;
55 await probe.stop(true);
56 return port;
57 }
58
5953 /** The line of what the child printed that says why: the first error, else the last line. */
6054 export function why(stderr: string): string {
6155 const lines = stderr
@@ -82,12 +76,19 @@ async function versionAt(port: number, probe: Record<string, string>): Promise<s
8276 }
8377 }
8478
85 /** Start the tree on `dataDir` and wait for it to answer health as `version`. */
79 /** Start the tree on `dataDir` and wait for it to answer health as `version`; on a fresh port if another process took the first. */
8680 export async function bootProof(input: BootInput): Promise<BootProof> {
8781 const entry = join(input.tree, "src", "main.ts");
8882 if (!existsSync(entry)) return { ok: false, line: `${input.tree} has no src/main.ts` };
83 return onFreshPort((port) => bootOnPort(input, entry, port));
84 }
85
86 async function bootOnPort(
87 input: BootInput,
88 entry: string,
89 port: number,
90 ): Promise<{ result: BootProof; portLost: boolean }> {
8991 const host = input.env ?? process.env;
90 const port = await freePort();
9192 const env: Record<string, string> = {
9293 PATH: host["PATH"] ?? "/usr/local/bin:/usr/bin:/bin",
9394 FORGE_DATA: input.dataDir,
@@ -121,17 +122,23 @@ export async function bootProof(input: BootInput): Promise<BootProof> {
121122 if (child.exitCode !== null) {
122123 await reading;
123124 const said = why(stderr);
124 return { ok: false, line: `exited with ${child.exitCode} before it answered health${said ? `: ${said}` : ""}` };
125 return {
126 result: { ok: false, line: `exited with ${child.exitCode} before it answered health${said ? `: ${said}` : ""}` },
127 portLost: lostPort(stderr),
128 };
125129 }
126130 const seen = await versionAt(port, probeHeaders(input.dataDir));
127 if (seen === input.version) return { ok: true, ms: Math.round(performance.now() - started) };
131 if (seen === input.version) return { result: { ok: true, ms: Math.round(performance.now() - started) }, portLost: false };
128132 if (seen !== undefined) last = `health answered as ${seen}`;
129133 await Bun.sleep(POLL_MS);
130134 }
131135 const said = why(stderr);
132136 return {
133 ok: false,
134 line: `did not answer health as ${input.version} within ${Math.round(budget / 1000)}s (${last})${said ? `: ${said}` : ""}`,
137 result: {
138 ok: false,
139 line: `did not answer health as ${input.version} within ${Math.round(budget / 1000)}s (${last})${said ? `: ${said}` : ""}`,
140 },
141 portLost: false,
135142 };
136143 } finally {
137144 child.kill("SIGKILL");
src/snapshot/boot.ts +26 −15
@@ -7,6 +7,7 @@
77 */
88
99 import { join } from "node:path";
10 import { lostPort, onFreshPort } from "../kernel/port.ts";
1011
1112 export interface Booted {
1213 readonly ok: boolean;
@@ -16,20 +17,22 @@ export interface Booted {
1617
1718 const TREE = join(import.meta.dir, "../..");
1819
19 async function freePort(): Promise<number> {
20 const probe = Bun.serve({ port: 0, fetch: () => new Response("") });
21 const port = probe.port ?? 0;
22 await probe.stop(true);
23 return port;
24 }
25
2620 export async function bootRestored(
2721 dataDir: string,
2822 budgetMs: number,
23 tree: string = TREE,
2924 ): Promise<Booted> {
30 const port = await freePort();
31 const child = Bun.spawn([process.execPath, join(TREE, "src/main.ts")], {
32 cwd: TREE,
25 return onFreshPort((port) => bootOnPort(dataDir, budgetMs, port, tree));
26 }
27
28 async function bootOnPort(
29 dataDir: string,
30 budgetMs: number,
31 port: number,
32 tree: string,
33 ): Promise<{ result: Booted; portLost: boolean }> {
34 const child = Bun.spawn([process.execPath, join(tree, "src/main.ts")], {
35 cwd: tree,
3336 env: {
3437 PATH: process.env["PATH"] ?? "",
3538 FORGE_DATA: dataDir,
@@ -42,27 +45,35 @@ export async function bootRestored(
4245 : {}),
4346 },
4447 stdout: "ignore",
45 stderr: "ignore",
48 stderr: "pipe",
4649 });
50 let stderr = "";
51 const reading = (async () => {
52 const decoder = new TextDecoder();
53 for await (const chunk of child.stderr) stderr = (stderr + decoder.decode(chunk, { stream: true })).slice(-8_192);
54 })().catch(() => undefined);
55 const no = (reason: string, portLost = false) => ({ result: { ok: false, reason }, portLost });
4756 const until = performance.now() + Math.min(budgetMs, 60_000);
4857 let last = "no answer";
4958 try {
5059 while (performance.now() < until) {
51 if (child.exitCode !== null)
52 return { ok: false, reason: `the restored forge exited with ${child.exitCode}` };
60 if (child.exitCode !== null) {
61 await reading;
62 return no(`the restored forge exited with ${child.exitCode}`, lostPort(stderr));
63 }
5364 try {
5465 const res = await fetch(`http://127.0.0.1:${port}/api/v1/health`, {
5566 signal: AbortSignal.timeout(2_000),
5667 });
5768 const body = (await res.json().catch(() => null)) as { ok?: unknown } | null;
58 if (res.ok && body?.ok === true) return { ok: true, reason: "" };
69 if (res.ok && body?.ok === true) return { result: { ok: true, reason: "" }, portLost: false };
5970 last = `health answered ${res.status}${body?.ok === false ? " not ok" : ""}`;
6071 } catch {
6172 last = "no answer yet";
6273 }
6374 await Bun.sleep(200);
6475 }
65 return { ok: false, reason: `the restored forge never read healthy: ${last}` };
76 return no(`the restored forge never read healthy: ${last}`);
6677 } finally {
6778 child.kill();
6879 await child.exited;
test/kernel-port.test.ts added +28
@@ -0,0 +1,28 @@
1 import { expect, test } from "bun:test";
2 import { PORT_RETRIES, freePort, lostPort, onFreshPort } from "../src/kernel/port.ts";
3
4 test("lostPort reads the ways a runtime says the address is taken", () => {
5 expect(lostPort("error: Failed to start server. Is port 3000 in use?")).toBe(true);
6 expect(lostPort("EADDRINUSE")).toBe(true);
7 expect(lostPort("TypeError: x is not a function")).toBe(false);
8 });
9
10 test("onFreshPort retries on a new port while the port is lost, then stops", async () => {
11 const seen: number[] = [];
12 const out = await onFreshPort(async (port) => {
13 seen.push(port);
14 return { result: seen.length, portLost: seen.length < 2 };
15 });
16 expect(out).toBe(2);
17 expect(seen.length).toBe(2);
18
19 let calls = 0;
20 await onFreshPort(async () => ({ result: ++calls, portLost: true }));
21 expect(calls).toBe(PORT_RETRIES + 1);
22 });
23
24 test("freePort answers a usable port number", async () => {
25 const port = await freePort();
26 expect(port).toBeGreaterThanOrEqual(1);
27 expect(port).toBeLessThanOrEqual(65535);
28 });
test/release-boot.test.ts +36 −1
@@ -8,9 +8,11 @@
88
99 import { afterAll, beforeAll, expect, test } from "bun:test";
1010 import { Database } from "bun:sqlite";
11 import { cpSync, existsSync, mkdirSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from "node:fs";
11 import { cpSync, existsSync, readFileSync, mkdirSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from "node:fs";
1212 import { tmpdir } from "node:os";
1313 import { join } from "node:path";
14 import { PORT_RETRIES } from "../src/kernel/port.ts";
15 import { bootRestored } from "../src/snapshot/boot.ts";
1416 import { bootProof, parseArgs, proveBoot, why } from "../src/release/boot.ts";
1517
1618 let root: string;
@@ -128,3 +130,36 @@ test("the build runs the proof on the image it built, and the chart runs it on a
128130 expect(hook).toContain("claimName: {{ .Release.Name }}-data");
129131 expect(hook).toContain("backoffLimit: 0");
130132 });
133
134 /** A tree whose main logs one line per run to `counter`, then dies saying `stderr`. */
135 function dying(name: string, stderr: string): { dir: string; runs: () => number } {
136 const counter = join(root, `${name}.count`);
137 const dir = tree(
138 name,
139 `import { appendFileSync } from "node:fs";\nappendFileSync(${JSON.stringify(counter)}, "run\\n");\nconsole.error(${JSON.stringify(stderr)});\nprocess.exit(1);\n`,
140 );
141 return { dir, runs: () => (existsSync(counter) ? readFileSync(counter, "utf8").split("\n").length - 1 : 0) };
142 }
143
144 test("bootProof starts again on a fresh port while the child lost its port, and gives up after PORT_RETRIES", async () => {
145 const t = dying("lost-proof", "error: address already in use");
146 const result = await bootProof({ tree: t.dir, version: "1.0.0", dataDir: root, budgetMs: 30_000 });
147 expect(result.ok).toBe(false);
148 expect(t.runs()).toBe(PORT_RETRIES + 1);
149 });
150
151 test("bootProof does not retry a child that died for another reason", async () => {
152 const t = dying("other-proof", "TypeError: x");
153 const result = await bootProof({ tree: t.dir, version: "1.0.0", dataDir: root, budgetMs: 30_000 });
154 expect(result.ok).toBe(false);
155 expect(t.runs()).toBe(1);
156 });
157
158 test("bootRestored retries a lost port and not another failure", async () => {
159 const lost = dying("lost-restored", "error: address already in use");
160 expect((await bootRestored(root, 30_000, lost.dir)).ok).toBe(false);
161 expect(lost.runs()).toBe(PORT_RETRIES + 1);
162 const other = dying("other-restored", "TypeError: x");
163 expect((await bootRestored(root, 30_000, other.dir)).ok).toBe(false);
164 expect(other.runs()).toBe(1);
165 });
test/spawn-pipes.test.ts +1
@@ -32,6 +32,7 @@ const PIPING: Record<string, { sites: string[]; reason: string }> = {
3232 "src/looks/capture.ts": { sites: ["stderr"], reason: "stderr read in a background loop; stdout ignored" },
3333 "src/release/devtools.ts": { sites: ["stderr"], reason: "stderr read for the DevTools address, then drained in a background loop; stdout ignored" },
3434 "src/release/boot.ts": { sites: ["stderr"], reason: "stderr read to its end in a background loop, awaited before the reason is read; stdout ignored" },
35 "src/snapshot/boot.ts": { sites: ["stderr"], reason: "stderr read to its end in a background loop, awaited before the exit is read for a lost port; stdout ignored" },
3536 "src/release/checkout.ts": { sites: ["stdout+stderr"], reason: "stdout and stderr both read via Response.text()" },
3637 "src/checks/install.ts": { sites: ["stdout+stderr"], reason: "prefillInstallCache reads stdout and stderr to the end via Response.text() while the exit is awaited" },
3738 "src/cli/config.ts": { sites: ["stdout"], reason: "stdout awaited with the exit; stderr ignored" },

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs