mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-02 21:32:10 +03:00
fix(cli): supervisor restarts on spontaneous exit-0 (OOM cgroup) + waits for port before respawn (#4425) (#4578)
Co-authored-by: Diego Rodrigues de Sa e Souza <diego.souza@cdwasolutions.com.br>
This commit is contained in:
committed by
GitHub
parent
f124806372
commit
fbcc43ce89
@@ -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;
|
||||
@@ -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;
|
||||
}
|
||||
@@ -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() {
|
||||
|
||||
56
bin/cli/runtime/supervisorPolicy.mjs
Normal file
56
bin/cli/runtime/supervisorPolicy.mjs
Normal file
@@ -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));
|
||||
}
|
||||
}
|
||||
@@ -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 {
|
||||
@@ -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
|
||||
@@ -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 () => {
|
||||
@@ -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 () => {
|
||||
@@ -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({
|
||||
@@ -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) ---
|
||||
|
||||
54
tests/unit/supervisor-policy-4425.test.ts
Normal file
54
tests/unit/supervisor-policy-4425.test.ts
Normal file
@@ -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);
|
||||
});
|
||||
Reference in New Issue
Block a user