Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions bin/cli/runtime/processSupervisor.mjs
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
import { spawn } from "node:child_process";
import { dirname } from "node:path";
import { writePidFile, cleanupPidFile, killAllSubprocesses } from "../utils/pid.mjs";
import {
RESTART_RESET_MS,
DEFAULT_MAX_RESTARTS,
shouldExitInsteadOfRestart,
computeRestartDelayMs,
waitUntilPortFree,
} from "./supervisorPolicy.mjs";

const CRASH_LOG_LINES = 50;
const RESTART_RESET_MS = 30_000;

export class ServerSupervisor {
constructor({ serverPath, env, maxRestarts = 2, memoryLimit = 512, onCrashCallback }) {
constructor({ serverPath, env, maxRestarts = DEFAULT_MAX_RESTARTS, memoryLimit = 512, onCrashCallback }) {
this.serverPath = serverPath;
this.env = env;
this.maxRestarts = maxRestarts;
Expand Down Expand Up @@ -54,7 +60,10 @@ export class ServerSupervisor {
const exitCode = typeof code === "number" ? code : null;
cleanupPidFile("server");

if (this.isShuttingDown || exitCode === 0) {
// #4425: only exit on an intentional shutdown. A spontaneous code-0 exit (e.g. a
// systemd MemoryMax cgroup kill, which reports the process exited cleanly) is anomalous
// and must be restarted, not treated as a graceful stop that leaves the gateway dead.
if (shouldExitInsteadOfRestart(this.isShuttingDown)) {
process.exit(exitCode ?? 0);
return;
}
Expand All @@ -79,12 +88,18 @@ export class ServerSupervisor {
}

this.restartCount++;
const delay = Math.min(1000 * 2 ** (this.restartCount - 1), 10_000);
const delay = computeRestartDelayMs(this.restartCount);
console.error(
`\n⚠ Server exited (code=${code ?? "?"}). Restarting in ${delay / 1000}s... (${this.restartCount}/${this.maxRestarts})`
);
if (this.crashLog.length) this.dumpCrashLog();
setTimeout(() => this.start(), delay);
// #4425: after a crash the OS may not have released the listen socket yet — restarting
// immediately produced the EADDRINUSE cascade that exhausted the restart budget. Wait
// (bounded) for the port to free up before respawning.
setTimeout(async () => {
await waitUntilPortFree(process.env.PORT || 20128);
this.start();
}, delay);
}

dumpCrashLog() {
Expand Down
56 changes: 56 additions & 0 deletions bin/cli/runtime/supervisorPolicy.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import net from "node:net";

// #4425: bumped from 30s — the old window reset the crash counter too quickly, so during
// an EADDRINUSE cascade the supervisor kept "recovering" then crashing within the window
// and exhausted its restart budget. A longer window keeps the counter meaningful.
export const RESTART_RESET_MS = 60_000;

// #4425: bumped from 2 — more recovery headroom before the supervisor gives up.
export const DEFAULT_MAX_RESTARTS = 3;

/**
* #4425: a clean child exit (code 0) is only intentional when the supervisor itself is
* shutting down. A spontaneous code-0 exit is anomalous — e.g. a systemd `MemoryMax`
* cgroup kill reports the process exited with code 0 — and MUST be restarted, not treated
* as a graceful stop (which left the gateway dead with `Restart=on-failure`).
*/
export function shouldExitInsteadOfRestart(isShuttingDown) {
return isShuttingDown === true;
}

/** Exponential backoff (1s, 2s, 4s, …) capped at 10s, matching the prior inline formula. */
export function computeRestartDelayMs(restartCount) {
return Math.min(1000 * 2 ** (Math.max(1, restartCount) - 1), 10_000);
}

/** Resolve true when nothing is listening on `port` (so a restart won't hit EADDRINUSE). */
export function isPortFree(port, host = "127.0.0.1") {
return new Promise((resolve) => {
const tester = net.createServer();
tester.once("error", (err) => {
// EADDRINUSE = something is bound → not free. Any other error → treat as free.
resolve(!(err && err.code === "EADDRINUSE"));
});
tester.once("listening", () => {
tester.close(() => resolve(true));
});
tester.listen(port, host);
});
}

/**
* #4425: wait until `port` is free before respawning. After a crash the OS may not have
* released the listen socket yet; restarting immediately produced the EADDRINUSE cascade
* that exhausted the restart budget. Polls up to `timeoutMs`, then proceeds anyway so a
* stuck port never blocks recovery forever.
*/
export async function waitUntilPortFree(port, timeoutMs = 10_000, intervalMs = 250) {
const p = Number(port);
if (!Number.isFinite(p) || p <= 0) return true;
const deadline = Date.now() + timeoutMs;
for (;;) {
if (await isPortFree(p)) return true;
if (Date.now() >= deadline) return false;
await new Promise((r) => setTimeout(r, intervalMs));
}
}
34 changes: 30 additions & 4 deletions tests/unit/cli-process-supervisor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import test from "node:test";
import assert from "node:assert/strict";
import { EventEmitter } from "node:events";

// #4425: the supervisor now waits for the listen port to free up before respawning.
// Point that probe at the no-op port 0 so the restart tests don't open real sockets.
process.env.PORT = "0";

// Stub para o processSupervisor: testa a lógica de restart/backoff/MITM sem processos reais.

class StubChild extends EventEmitter {
Expand Down Expand Up @@ -41,9 +45,13 @@ test("detectMitmCrash retorna false com menos de 2 sinais", async () => {

// --- ServerSupervisor: lógica de restart ---

test("ServerSupervisor.handleExit com code=0 chama process.exit(0)", async () => {
test("ServerSupervisor.handleExit com code=0 espontâneo reinicia em vez de sair (#4425)", async () => {
const { ServerSupervisor } = await import("../../bin/cli/runtime/processSupervisor.mjs");

// waitUntilPortFree no-ops on port 0, so the scheduled restart fires fast and leaks no timer.
const origPort = process.env.PORT;
process.env.PORT = "0";

const exits: number[] = [];
const origExit = process.exit.bind(process);
// @ts-ignore
Expand All @@ -56,11 +64,27 @@ test("ServerSupervisor.handleExit com code=0 chama process.exit(0)", async () =>
env: {},
maxRestarts: 2,
});
let started = 0;
// Stub start() so the scheduled restart never spawns the fake server.
supervisor.start = () => {
started++;
return null as any;
};
supervisor.handleExit(0);

// #4425: a spontaneous code-0 exit (e.g. a systemd MemoryMax cgroup kill, which reports
// a clean exit) must NOT terminate the supervisor — it schedules a restart instead.
assert.equal(exits.length, 0, "must not process.exit on a spontaneous code-0 exit");
assert.equal(supervisor.restartCount, 1);

// Let the scheduled restart fire so no timer leaks past the test.
await new Promise((r) => setTimeout(r, 1100));
assert.equal(started, 1, "restart should fire after the backoff delay");

// @ts-ignore
process.exit = origExit;
assert.equal(exits[0], 0);
if (origPort === undefined) delete process.env.PORT;
else process.env.PORT = origPort;
});

test("ServerSupervisor.handleExit com isShuttingDown=true chama process.exit imediato", async () => {
Expand Down Expand Up @@ -125,6 +149,7 @@ test("ServerSupervisor.handleExit exibe crash log ao reiniciar", async () => {

console.error = origErr;
assert.ok(logs.some((l) => l.includes("line1") || l.includes("crash log")));
await new Promise((r) => setTimeout(r, 1100)); // drain the scheduled restart timer
});

test("ServerSupervisor chama onCrashCallback após maxRestarts atingido", async () => {
Expand Down Expand Up @@ -179,7 +204,7 @@ test("ServerSupervisor retorna 'disable-mitm-and-retry' chama start() novamente"
assert.equal(supervisor.restartCount, 0); // foi resetado
});

test("ServerSupervisor reseta restartCount após processo viver >=30s", async () => {
test("ServerSupervisor reseta restartCount após viver >= RESTART_RESET_MS (#4425: 60s)", async () => {
const { ServerSupervisor } = await import("../../bin/cli/runtime/processSupervisor.mjs");

const supervisor = new ServerSupervisor({
Expand All @@ -189,10 +214,11 @@ test("ServerSupervisor reseta restartCount após processo viver >=30s", async ()
});
supervisor.start = () => null as any;
supervisor.restartCount = 2;
supervisor.startedAt = Date.now() - 31_000; // viveu 31s
supervisor.startedAt = Date.now() - 61_000; // #4425: reset window bumped 30s→60s
supervisor.handleExit(1);

assert.equal(supervisor.restartCount, 1); // reset p/ 0, depois incrementado p/ 1
await new Promise((r) => setTimeout(r, 1100)); // drain the scheduled restart timer
});

// --- Node.js v24 compat: process.exit() must receive a number (#3748) ---
Expand Down
54 changes: 54 additions & 0 deletions tests/unit/supervisor-policy-4425.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import test from "node:test";
import assert from "node:assert/strict";
import net from "node:net";

// #4425: the supervisor treated a clean exit (code 0) as intentional and exited instead
// of restarting — but a systemd MemoryMax cgroup kill reports code 0, so the OOM'd gateway
// stayed dead. And it restarted immediately after a crash, hitting EADDRINUSE before the
// OS released the port. supervisorPolicy centralizes the restart decision + a port-wait.

const {
RESTART_RESET_MS,
DEFAULT_MAX_RESTARTS,
shouldExitInsteadOfRestart,
computeRestartDelayMs,
isPortFree,
waitUntilPortFree,
} = await import("../../bin/cli/runtime/supervisorPolicy.mjs");

test("#4425 spontaneous code-0 exit restarts (only shutdown exits)", () => {
assert.equal(shouldExitInsteadOfRestart(false), false); // OOM cgroup code-0 → restart
assert.equal(shouldExitInsteadOfRestart(true), true); // operator stop() → exit
});

test("#4425 tuned recovery constants", () => {
assert.equal(RESTART_RESET_MS, 60_000);
assert.equal(DEFAULT_MAX_RESTARTS, 3);
});

test("#4425 restart backoff is 1s,2s,4s… capped at 10s", () => {
assert.equal(computeRestartDelayMs(1), 1000);
assert.equal(computeRestartDelayMs(2), 2000);
assert.equal(computeRestartDelayMs(3), 4000);
assert.equal(computeRestartDelayMs(10), 10_000);
});

test("#4425 isPortFree detects a bound port and waitUntilPortFree resolves once released", async () => {
const server = net.createServer();
await new Promise<void>((resolve) => server.listen(0, "127.0.0.1", () => resolve()));
const port = (server.address() as net.AddressInfo).port;

assert.equal(await isPortFree(port), false, "bound port is not free");

// waitUntilPortFree should time out (return false) while the port stays bound.
assert.equal(await waitUntilPortFree(port, 300, 50), false);

await new Promise<void>((resolve) => server.close(() => resolve()));
assert.equal(await isPortFree(port), true, "released port is free");
assert.equal(await waitUntilPortFree(port, 1000, 50), true);
});

test("#4425 waitUntilPortFree no-ops on an invalid port", async () => {
assert.equal(await waitUntilPortFree(undefined), true);
assert.equal(await waitUntilPortFree(0), true);
});
Loading