diff --git a/scripts/gate-lease.mjs b/scripts/gate-lease.mjs index ca11a3ca..d4eea50c 100644 --- a/scripts/gate-lease.mjs +++ b/scripts/gate-lease.mjs @@ -460,44 +460,49 @@ export async function runUnderLease({ log( `gate-lease: could not release the lease (${err?.message ?? err}). A waiter takes it over once it is ${formatDuration(STALE_MS)} stale — unless whatever blocked this removal persists, in which case remove ${lockPathOf(dir)} by hand.`, ); + // Not "released after": the lock may still be blocking the queue. + return; } log( `gate-lease: released after ${formatDuration(Date.now() - startedRunning)}${waited >= pollMs ? ` (waited ${formatDuration(waited)} first)` : ""}.`, ); }; - const spec = spawnSpec(command, args); - const child = spawn(spec.command, spec.args, { - stdio, - env, - shell: spec.shell, - // Its own group, so one signal reaches every descendant. See the header. - detached: spec.detached, - }); - const signals = ["SIGINT", "SIGTERM", "SIGHUP"]; let stoppedBy = null; let escalation = null; - const onSignal = (signal) => { - if (stoppedBy !== null) return; - stoppedBy = signal; - log(`gate-lease: received ${signal}; stopping the gate.`); - signalTree(child, signal); - escalation = setTimeout(() => { - log( - `gate-lease: the gate did not exit within ${graceMs}ms of ${signal}; sending SIGKILL.`, - ); - signalTree(child, "SIGKILL"); - }, graceMs); - }; - const handlers = signals.map((signal) => { - const handler = () => onSignal(signal); - process.on(signal, handler); - return [signal, handler]; - }); - + let handlers = []; let outcome; + // Everything from here to the child's exit is inside the `try`, so a + // synchronous failure to even start it — an argument `winShellArgs` + // refuses, a spawn option Node rejects — releases the lease the same way + // an asynchronous spawn error does. try { + const spec = spawnSpec(command, args); + const child = spawn(spec.command, spec.args, { + stdio, + env, + shell: spec.shell, + // Its own group, so one signal reaches every descendant. See the header. + detached: spec.detached, + }); + const onSignal = (signal) => { + if (stoppedBy !== null) return; + stoppedBy = signal; + log(`gate-lease: received ${signal}; stopping the gate.`); + signalTree(child, signal); + escalation = setTimeout(() => { + log( + `gate-lease: the gate did not exit within ${graceMs}ms of ${signal}; sending SIGKILL.`, + ); + signalTree(child, "SIGKILL"); + }, graceMs); + }; + handlers = signals.map((signal) => { + const handler = () => onSignal(signal); + process.on(signal, handler); + return [signal, handler]; + }); outcome = await new Promise((resolve, reject) => { child.once("error", reject); child.once("exit", (code, signal) => resolve({ code, signal })); diff --git a/scripts/gate-lease.test.mjs b/scripts/gate-lease.test.mjs index f2b9933a..494c37e3 100644 --- a/scripts/gate-lease.test.mjs +++ b/scripts/gate-lease.test.mjs @@ -274,6 +274,27 @@ test("a command that cannot be spawned still releases the lease", async () => { assert.ok(!existsSync(leaseTarget(dir))); }); +test("a command that fails synchronously before it starts still releases the lease", async () => { + const dir = freshDir(); + const { log } = collectLog(); + // `spawn` validates its arguments synchronously: a non-array `args` throws + // before any child exists, the same class as `winShellArgs` refusing a `%` + // on Windows. That throw must not strand the lock until stale takeover. + await assert.rejects( + runUnderLease({ + command: process.execPath, + args: "-e", + dir, + log, + stdio: "ignore", + pollMs: 25, + }), + { code: "ERR_INVALID_ARG_TYPE" }, + ); + assert.ok(!existsSync(lockPathOf(dir))); + assert.ok(!existsSync(leaseTarget(dir))); +}); + test("a lock replaced mid-run (stale takeover) is the winner's, and is left alone", async () => { const dir = freshDir(); const { lines, log } = collectLog();