diff --git a/internal/_serve-reaped.mjs b/internal/_serve-reaped.mjs new file mode 100644 index 000000000..95395de20 --- /dev/null +++ b/internal/_serve-reaped.mjs @@ -0,0 +1,62 @@ +/** + * _serve-reaped.mjs — spawn a dev server that cannot outlive the process that started it. + * + * THE LEAK, MEASURED. Twelve gates in this repo spawn `serve-strict`; eleven of them reap it + * only on the happy path — `srv.kill()` at the end of the script. That works until the script + * does not reach the end. When the box ran low on memory it killed several gate processes + * mid-run, and every server those gates had spawned was orphaned. Counted at the time: ten + * orphans in one worktree, four in another. Each orphan holds memory, which makes the next + * kill more likely, which orphans more servers. The leak feeds the condition that causes it. + * + * I caught this by leaking two myself, minutes after describing the loop to someone else — + * so "my gate kills its server" is not evidence. The kill has to survive the killing. + * + * WHAT THIS DOES. Registers the child against SIGTERM, SIGINT, SIGHUP and normal exit, and + * unregisters cleanly when you stop it yourself. `exit` fires for a plain process.exit() and + * for an uncaught throw; the signals cover the memory killer and Ctrl-C. Nothing covers + * SIGKILL — nothing can — which is why the port is also reported, so a sweep can find it. + * + * const srv = serveReaped(PUBLIC_DIR, port) + * ... use it ... + * srv.stop() // idempotent; safe to call from a finally block + */ +import { spawn } from 'node:child_process' +import { resolve, dirname } from 'node:path' +import { fileURLToPath } from 'node:url' + +const HERE = dirname(fileURLToPath(import.meta.url)) + +/** + * @param {string} directory what to serve + * @param {number} port + * @returns {{ child: import('node:child_process').ChildProcess, port: number, stop: () => void }} + */ +export function serveReaped(directory, port) { + const child = spawn( + process.execPath, + [resolve(HERE, 'serve-strict.mjs'), '--port', String(port), '--directory', directory], + { stdio: 'ignore' }, + ) + + let stopped = false + const stop = () => { + if (stopped) return + stopped = true + try { child.kill() } catch { /* already gone */ } + process.off('exit', onExit) + for (const sig of SIGNALS) process.off(sig, onSignal) + } + + const onExit = () => { try { child.kill() } catch { /* already gone */ } } + /* Re-raise after cleaning up, so the caller still sees the signal's exit status. */ + const onSignal = (sig) => { + onExit() + process.exit(sig === 'SIGINT' ? 130 : 143) + } + const SIGNALS = ['SIGTERM', 'SIGINT', 'SIGHUP'] + + process.on('exit', onExit) + for (const sig of SIGNALS) process.on(sig, onSignal) + + return { child, port, stop } +} diff --git a/internal/check-embed-basemap-paints.mjs b/internal/check-embed-basemap-paints.mjs index 4c7118bca..ea179dd2d 100644 --- a/internal/check-embed-basemap-paints.mjs +++ b/internal/check-embed-basemap-paints.mjs @@ -27,7 +27,7 @@ * still paint nothing, and a screenshot is the only witness to that. */ import { chromium } from 'playwright' -import { spawn } from 'node:child_process' +import { serveReaped } from './_serve-reaped.mjs' import { writeFileSync, mkdtempSync, readFileSync } from 'node:fs' import { resolve, dirname } from 'node:path' import { tmpdir } from 'node:os' @@ -46,11 +46,12 @@ const isBasemap = (u) => /api\.mapbox\.com\/(styles|v4|fonts|tilesets)|tiles\.mapbox\.com/.test(u) && !/events\.mapbox\.com/.test(u) -function serve(dir, port) { - return spawn(process.execPath, [resolve(REPO, 'internal/serve-strict.mjs'), '--port', String(port), '--directory', dir], { - stdio: 'ignore', - }) -} +/* Reaped on signals as well as on the happy path. This gate used to kill its two servers + only at the end, which is fine until the end is not reached: when the box ran low on + memory it killed gate processes mid-run and their servers were orphaned, holding memory + and making the next kill likelier. Measured both ways before this changed — the old + spawn survived a SIGTERM to its parent, this one does not. */ +const serve = (dir, port) => serveReaped(dir, port) async function waitFor(url, ms = 20000) { const until = Date.now() + ms @@ -74,8 +75,8 @@ writeFileSync( const leafSrv = serve(PUBLIC, leafPort) const hostSrv = serve(hostDir, hostPort) const stop = () => { - try { leafSrv.kill() } catch {} - try { hostSrv.kill() } catch {} + leafSrv.stop() + hostSrv.stop() } if (!(await waitFor(`http://127.0.0.1:${leafPort}/${LEAF}`)) || !(await waitFor(`http://127.0.0.1:${hostPort}/index.html`))) {