Skip to content

Commit e7bcd2a

Browse files
goldmedalgoldmedal
committed
fix(genbi): bound installed-package cleanup
1 parent f876798 commit e7bcd2a

5 files changed

Lines changed: 299 additions & 33 deletions

File tree

‎apps/genbi/scripts/installed-package-acceptance.mjs‎

Lines changed: 21 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,13 @@
99
* minimal PATH and a require hook that turns any source-checkout read into a
1010
* hard failure, so a checkout-only fallback cannot make this pass.
1111
*/
12-
import { spawn } from "node:child_process";
1312
import { existsSync } from "node:fs";
1413
import { mkdtemp, mkdir, readdir, rm, writeFile } from "node:fs/promises";
1514
import { createServer as createHttpServer } from "node:http";
1615
import { createServer } from "node:net";
1716
import os from "node:os";
1817
import path from "node:path";
18+
import { closeServerBounded, runBounded, spawnProcessGroup, stopProcessTree } from "./process-cleanup.mjs";
1919
import { readSseFrames } from "./sse-frames.mjs";
2020

2121
const packageRoot = path.resolve(process.cwd());
@@ -27,15 +27,18 @@ const sourceAuditHook = path.join(tempRoot, "block-checkout-access.cjs");
2727
const port = await reservePort();
2828
const fixtureProvider = await startSetupFixtureProvider();
2929
let serverProcess;
30+
let phase = "initialize";
3031

3132
try {
33+
markPhase("pack");
3234
await Promise.all([mkdir(packDirectory), mkdir(installRoot), mkdir(workspaceRoot)]);
3335
await run("pnpm", ["pack", "--pack-destination", packDirectory], { cwd: packageRoot });
3436

3537
const packageTarball = await onlyTarball(packDirectory);
3638
const packedFiles = await tarFiles(packageTarball);
3739
assertPublishedFiles(packedFiles);
3840

41+
markPhase("install");
3942
await run("npm", ["init", "--yes"], { cwd: installRoot });
4043
await run("npm", ["install", "--no-audit", "--no-fund", packageTarball], { cwd: installRoot });
4144

@@ -49,20 +52,23 @@ try {
4952
}
5053
}
5154

55+
markPhase("start");
5256
await writeFile(sourceAuditHook, createSourceAuditHook(), { mode: 0o600 });
5357
const childEnv = controlledEnvironment({ installRoot, workspaceRoot, port, sourceAuditHook, fixtureEndpoint: fixtureProvider.endpoint });
54-
serverProcess = spawn("npx", ["--no-install", "genbi"], {
58+
serverProcess = spawnProcessGroup("npx", ["--no-install", "genbi"], {
5559
cwd: installRoot,
5660
env: childEnv,
5761
stdio: ["ignore", "pipe", "pipe"],
5862
});
5963
const output = collectOutput(serverProcess);
6064

65+
markPhase("setup-connect");
6166
await waitForServer(port, output);
6267
await verifyFirstRunSetup(port, workspaceRoot, fixtureProvider);
63-
await stop(serverProcess);
68+
await stopProcessTree(serverProcess);
6469
serverProcess = undefined;
6570

71+
markPhase("complete");
6672
process.stdout.write(`${JSON.stringify({
6773
ok: true,
6874
checks: [
@@ -72,8 +78,14 @@ try {
7278
],
7379
}, null, 2)}\n`);
7480
} finally {
75-
if (serverProcess) await stop(serverProcess).catch(() => undefined);
76-
await Promise.all([closeServer(fixtureProvider.server), rm(tempRoot, { recursive: true, force: true })]);
81+
const cleanupErrors = [];
82+
markPhase("cleanup-child");
83+
if (serverProcess) await stopProcessTree(serverProcess).catch((error) => cleanupErrors.push(error));
84+
markPhase("cleanup-fixture");
85+
await closeServerBounded(fixtureProvider.server).catch((error) => cleanupErrors.push(error));
86+
markPhase("cleanup-temp");
87+
await rm(tempRoot, { recursive: true, force: true }).catch((error) => cleanupErrors.push(error));
88+
if (cleanupErrors.length > 0) throw new AggregateError(cleanupErrors, `installed package cleanup failed during ${phase}`);
7789
}
7890

7991
function controlledEnvironment({ installRoot, workspaceRoot, port: selectedPort, sourceAuditHook: hook, fixtureEndpoint }) {
@@ -291,20 +303,6 @@ function collectOutput(child) {
291303
return { get exitCode() { return exitCode; }, text: () => `${stdout}${stderr}`.slice(-4_000) };
292304
}
293305

294-
async function stop(child) {
295-
if (child.exitCode !== null || child.signalCode !== null) return;
296-
child.kill("SIGTERM");
297-
await new Promise((resolve) => {
298-
const forceKill = setTimeout(() => {
299-
if (child.exitCode === null && child.signalCode === null) child.kill("SIGKILL");
300-
}, 5_000);
301-
child.once("exit", () => {
302-
clearTimeout(forceKill);
303-
resolve();
304-
});
305-
});
306-
}
307-
308306
async function onlyTarball(directory) {
309307
const files = (await readdir(directory)).filter((file) => file.endsWith(".tgz"));
310308
if (files.length !== 1) throw new Error(`expected one package tarball, found ${JSON.stringify(files)}`);
@@ -327,17 +325,7 @@ function assertPublishedFiles(files) {
327325
}
328326

329327
function run(command, args, options) {
330-
return new Promise((resolve, reject) => {
331-
const child = spawn(command, args, { ...options, stdio: ["ignore", "pipe", "pipe"] });
332-
let stdout = "";
333-
let stderr = "";
334-
child.stdout.setEncoding("utf8");
335-
child.stderr.setEncoding("utf8");
336-
child.stdout.on("data", (chunk) => (stdout += chunk));
337-
child.stderr.on("data", (chunk) => (stderr += chunk));
338-
child.once("error", reject);
339-
child.once("exit", (code) => code === 0 ? resolve({ stdout, stderr }) : reject(new Error(`${command} ${args.join(" ")} failed (${code}): ${stderr.slice(-4_000)}`)));
340-
});
328+
return runBounded(command, args, options);
341329
}
342330

343331
async function reservePort() {
@@ -356,7 +344,7 @@ function delay(milliseconds) {
356344
return new Promise((resolve) => setTimeout(resolve, milliseconds));
357345
}
358346

359-
function closeServer(server) {
360-
if (!server.listening) return Promise.resolve();
361-
return new Promise((resolve, reject) => server.close((error) => error ? reject(error) : resolve()));
347+
function markPhase(nextPhase) {
348+
phase = nextPhase;
349+
process.stderr.write(`[installed-package] phase=${phase}\n`);
362350
}

‎apps/genbi/scripts/process-cleanup.d.mts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import type { ChildProcess, SpawnOptions } from "node:child_process";
2+
import type { Server } from "node:net";
3+
4+
export function spawnProcessGroup(command: string, args: readonly string[], options?: SpawnOptions): ChildProcess;
5+
export function stopProcessTree(child: ChildProcess, options?: { graceMs?: number; forceMs?: number }): Promise<void>;
6+
export function runBounded(command: string, args: readonly string[], options?: SpawnOptions & { timeoutMs?: number }): Promise<{ stdout: string; stderr: string }>;
7+
export function closeServerBounded(server: Server, options?: { timeoutMs?: number; forceMs?: number }): Promise<void>;

‎apps/genbi/scripts/process-cleanup.mjs‎

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
import { spawn } from "node:child_process";
2+
3+
const DEFAULT_GRACE_MS = 5_000;
4+
const DEFAULT_FORCE_MS = 2_000;
5+
const DEFAULT_COMMAND_TIMEOUT_MS = 120_000;
6+
const DEFAULT_CLOSE_TIMEOUT_MS = 5_000;
7+
8+
/** Spawn a command in its own POSIX process group so descendants can be stopped together. */
9+
export function spawnProcessGroup(command, args, options = {}) {
10+
return spawn(command, args, {
11+
...options,
12+
// Windows does not support negative-PID process-group signaling. The
13+
// packaged gate's supported evidence runners are POSIX, while this keeps
14+
// the fallback behavior valid if the script is invoked elsewhere.
15+
detached: process.platform !== "win32",
16+
});
17+
}
18+
19+
/** Stop a spawned command and every descendant, with finite TERM/KILL bounds. */
20+
export async function stopProcessTree(child, { graceMs = DEFAULT_GRACE_MS, forceMs = DEFAULT_FORCE_MS } = {}) {
21+
if (!child.pid) return;
22+
if (process.platform === "win32") {
23+
await stopSingleProcess(child, { graceMs, forceMs });
24+
return;
25+
}
26+
27+
const groupId = child.pid;
28+
if (!processGroupAlive(groupId)) return;
29+
signalGroup(groupId, "SIGTERM");
30+
if (await waitForProcessGroupExit(groupId, graceMs)) return;
31+
signalGroup(groupId, "SIGKILL");
32+
if (await waitForProcessGroupExit(groupId, forceMs)) return;
33+
throw new Error(`process group ${groupId} did not exit after ${graceMs + forceMs}ms`);
34+
}
35+
36+
/** Run a bounded command, retaining a diagnostic tail if it exits non-zero. */
37+
export function runBounded(command, args, { timeoutMs = DEFAULT_COMMAND_TIMEOUT_MS, ...options } = {}) {
38+
return new Promise((resolve, reject) => {
39+
const child = spawnProcessGroup(command, args, { ...options, stdio: ["ignore", "pipe", "pipe"] });
40+
let stdout = "";
41+
let stderr = "";
42+
let settled = false;
43+
let timer;
44+
const finish = (callback) => {
45+
if (settled) return;
46+
settled = true;
47+
clearTimeout(timer);
48+
callback();
49+
};
50+
child.stdout.setEncoding("utf8");
51+
child.stderr.setEncoding("utf8");
52+
child.stdout.on("data", (chunk) => (stdout += chunk));
53+
child.stderr.on("data", (chunk) => (stderr += chunk));
54+
child.once("error", (error) => finish(() => reject(error)));
55+
child.once("exit", (code) => {
56+
finish(() => code === 0
57+
? resolve({ stdout, stderr })
58+
: reject(new Error(`${command} ${args.join(" ")} failed (${code}): ${stderr.slice(-4_000)}`)));
59+
});
60+
timer = setTimeout(() => {
61+
if (settled) return;
62+
settled = true;
63+
void stopProcessTree(child).then(
64+
() => reject(new Error(`${command} ${args.join(" ")} exceeded ${timeoutMs}ms`)),
65+
(error) => reject(new AggregateError([error], `${command} ${args.join(" ")} timed out and cleanup failed`)),
66+
);
67+
}, timeoutMs);
68+
});
69+
}
70+
71+
/** Close a fixture listener without allowing an open client to stall cleanup forever. */
72+
export async function closeServerBounded(server, { timeoutMs = DEFAULT_CLOSE_TIMEOUT_MS, forceMs = DEFAULT_FORCE_MS } = {}) {
73+
if (!server.listening) return;
74+
const closed = new Promise((resolve, reject) => {
75+
server.close((error) => error ? reject(error) : resolve());
76+
});
77+
if (await settlesWithin(closed, timeoutMs)) return;
78+
// The listener is already closed to new connections; now release the clients
79+
// that prevented its close callback, then retain a second finite bound.
80+
server.closeAllConnections?.();
81+
if (await settlesWithin(closed, forceMs)) return;
82+
throw new Error(`fixture server did not close after ${timeoutMs + forceMs}ms`);
83+
}
84+
85+
function signalGroup(groupId, signal) {
86+
try {
87+
process.kill(-groupId, signal);
88+
} catch (error) {
89+
if (error?.code !== "ESRCH") throw error;
90+
}
91+
}
92+
93+
function processGroupAlive(groupId) {
94+
try {
95+
process.kill(-groupId, 0);
96+
return true;
97+
} catch (error) {
98+
if (error?.code === "ESRCH") return false;
99+
throw error;
100+
}
101+
}
102+
103+
async function waitForProcessGroupExit(groupId, timeoutMs) {
104+
const deadline = Date.now() + timeoutMs;
105+
while (Date.now() < deadline) {
106+
if (!processGroupAlive(groupId)) return true;
107+
await delay(25);
108+
}
109+
return !processGroupAlive(groupId);
110+
}
111+
112+
async function stopSingleProcess(child, { graceMs, forceMs }) {
113+
if (child.exitCode !== null || child.signalCode !== null) return;
114+
child.kill("SIGTERM");
115+
if (await waitForChildExit(child, graceMs)) return;
116+
child.kill("SIGKILL");
117+
if (await waitForChildExit(child, forceMs)) return;
118+
throw new Error(`process ${child.pid ?? "unknown"} did not exit after ${graceMs + forceMs}ms`);
119+
}
120+
121+
function waitForChildExit(child, timeoutMs) {
122+
if (child.exitCode !== null || child.signalCode !== null) return Promise.resolve(true);
123+
return new Promise((resolve) => {
124+
const timer = setTimeout(() => resolve(false), timeoutMs);
125+
child.once("exit", () => {
126+
clearTimeout(timer);
127+
resolve(true);
128+
});
129+
});
130+
}
131+
132+
async function settlesWithin(promise, timeoutMs) {
133+
const timedOut = Symbol("timed out");
134+
const result = await Promise.race([
135+
promise.then(() => undefined),
136+
delay(timeoutMs).then(() => timedOut),
137+
]);
138+
return result !== timedOut;
139+
}
140+
141+
function delay(milliseconds) {
142+
return new Promise((resolve) => setTimeout(resolve, milliseconds));
143+
}

0 commit comments

Comments
 (0)