mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-27 22:15:33 +00:00
fix(serve): boot-readiness deadline releases PGLite lock on wedged boot (#3273)
A serve process that wedges mid-boot (e.g. a boot step blocked on an unreachable upstream) held the PGLite write lock indefinitely — the post-#2348 lock discipline never steals from a live holder, so every CLI consumer timed out until the serve PID was manually killed. runServe (stdio path) now arms a boot-readiness deadline around startMcpServer: if the transport hasn't connected within GBRAIN_SERVE_BOOT_TIMEOUT_SECONDS (default 60, 0 disables), it logs the condition, awaits engine.disconnect() (raced against the existing 5s cleanup deadline so a wedged WASM close can't trap it either), and exits non-zero so supervisors restart with backoff. A completed boot clears the timer; the HTTP path is untouched (own lifecycle). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
ca47c054b8
commit
f1f6fcbeb6
+68
-1
@@ -9,6 +9,17 @@ import { startMcpServer } from '../mcp/server.ts';
|
||||
// the dir, sees a dead PID, and removes it).
|
||||
const CLEANUP_DEADLINE_MS = 5_000;
|
||||
|
||||
// Boot-readiness deadline (#3273). A serve process that wedges mid-boot
|
||||
// (e.g. an MCP boot step that never completes because a configured
|
||||
// upstream is unreachable) holds the PGLite write lock indefinitely: the
|
||||
// post-#2348 lock discipline never steals from a live holder, so every
|
||||
// CLI consumer times out until someone hunts down and kills the PID. If
|
||||
// startMcpServer hasn't finished connecting the transport within this
|
||||
// window, we release the engine (dropping the lock) and exit non-zero so
|
||||
// a supervisor can restart with backoff. Env-tunable via
|
||||
// GBRAIN_SERVE_BOOT_TIMEOUT_SECONDS; 0 disables.
|
||||
const DEFAULT_BOOT_TIMEOUT_SECONDS = 60;
|
||||
|
||||
// How often the parent-process watchdog polls the live kernel parent PID
|
||||
// (via `readLiveParentPid`, NOT the cached `process.ppid` — see that
|
||||
// helper's comment). We don't receive a signal when our parent dies (the
|
||||
@@ -67,6 +78,10 @@ export interface ServeOptions {
|
||||
// transport.onclose still cover legitimate shutdown.
|
||||
// Defaults to `process.env.MCP_STDIO === '1'` when omitted.
|
||||
mcpStdio?: boolean;
|
||||
// Test seam for the boot-readiness deadline (#3273). Milliseconds.
|
||||
// Defaults to GBRAIN_SERVE_BOOT_TIMEOUT_SECONDS (seconds; 60 when
|
||||
// unset, 0 disables) when omitted.
|
||||
bootTimeoutMs?: number;
|
||||
}
|
||||
|
||||
export async function runServe(
|
||||
@@ -142,7 +157,43 @@ export async function runServe(
|
||||
installStdioLifecycle(engine, args, opts);
|
||||
|
||||
const start = opts.startMcpServer ?? startMcpServer;
|
||||
await start(engine);
|
||||
|
||||
// Boot-readiness deadline (#3273): never sit on the PGLite write lock
|
||||
// forever with a boot that never completes. On expiry: log, release the
|
||||
// engine (drops the lock), exit non-zero so supervisors restart with
|
||||
// backoff. The disconnect itself is raced against CLEANUP_DEADLINE_MS,
|
||||
// same as the graceful-shutdown path, so a wedged WASM close can't trap
|
||||
// us either.
|
||||
const bootTimeoutMs = opts.bootTimeoutMs ?? resolveBootTimeoutMs();
|
||||
let bootDeadline: ReturnType<typeof setTimeout> | null = null;
|
||||
if (bootTimeoutMs > 0) {
|
||||
const log = opts.log ?? ((msg: string) => console.error(msg));
|
||||
const exit = opts.exit ?? ((code?: number) => { process.exit(code); });
|
||||
bootDeadline = setTimeout(() => {
|
||||
log(
|
||||
`GBrain MCP server: boot did not complete within ${bootTimeoutMs}ms — releasing DB lock and exiting so other consumers unblock (check configured provider endpoints; tune via GBRAIN_SERVE_BOOT_TIMEOUT_SECONDS, 0 disables)`,
|
||||
);
|
||||
const cleanup = setTimeout(() => { exit(1); }, CLEANUP_DEADLINE_MS);
|
||||
cleanup.unref?.();
|
||||
Promise.resolve()
|
||||
.then(() => engine.disconnect())
|
||||
.catch((err: unknown) => {
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
log(`GBrain MCP server: boot-deadline cleanup error: ${msg}`);
|
||||
})
|
||||
.finally(() => {
|
||||
clearTimeout(cleanup);
|
||||
exit(1);
|
||||
});
|
||||
}, bootTimeoutMs);
|
||||
bootDeadline.unref?.();
|
||||
}
|
||||
|
||||
try {
|
||||
await start(engine);
|
||||
} finally {
|
||||
if (bootDeadline) clearTimeout(bootDeadline);
|
||||
}
|
||||
// startMcpServer's `await server.connect(transport)` resolves once the
|
||||
// SDK has wired up its stdin 'data' listener; that listener keeps the
|
||||
// event loop alive. We deliberately do NOT add `await new Promise(() =>
|
||||
@@ -150,6 +201,22 @@ export async function runServe(
|
||||
// hooks from being able to call process.exit() cleanly.
|
||||
}
|
||||
|
||||
// Env resolution for the boot deadline. Lenient (warn + default) rather
|
||||
// than throw: this is an incident-time escape hatch, and a typo'd env var
|
||||
// must not turn a boot-safety net into a boot failure of its own.
|
||||
function resolveBootTimeoutMs(): number {
|
||||
const raw = process.env.GBRAIN_SERVE_BOOT_TIMEOUT_SECONDS;
|
||||
if (raw === undefined || raw.trim() === '') return DEFAULT_BOOT_TIMEOUT_SECONDS * 1000;
|
||||
const n = Number(raw);
|
||||
if (!Number.isFinite(n) || n < 0) {
|
||||
console.error(
|
||||
`[gbrain serve] ignoring invalid GBRAIN_SERVE_BOOT_TIMEOUT_SECONDS=${JSON.stringify(raw)} — using default ${DEFAULT_BOOT_TIMEOUT_SECONDS}s`,
|
||||
);
|
||||
return DEFAULT_BOOT_TIMEOUT_SECONDS * 1000;
|
||||
}
|
||||
return n * 1000;
|
||||
}
|
||||
|
||||
interface StdioLifecycleDeps {
|
||||
stdin: NodeJS.ReadableStream & { isTTY?: boolean };
|
||||
signals: Pick<NodeJS.Process, 'on'>;
|
||||
|
||||
@@ -561,3 +561,44 @@ describe('watchdog platform defaults', () => {
|
||||
expect(n).toBeGreaterThanOrEqual(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('boot-readiness deadline (#3273)', () => {
|
||||
test('a boot that never completes releases the engine and exits non-zero', async () => {
|
||||
const h = makeHarness();
|
||||
// Never-resolving boot = serve wedged mid-boot while holding the
|
||||
// PGLite write lock (the reported symptom: every CLI consumer times
|
||||
// out on the lock until the serve PID is manually killed).
|
||||
h.opts.startMcpServer = () => new Promise<void>(() => {});
|
||||
h.opts.bootTimeoutMs = 20;
|
||||
void runServe(h.engine as unknown as BrainEngine, [], h.opts);
|
||||
|
||||
const code = await h.exited;
|
||||
expect(code).toBe(1);
|
||||
expect(h.engine.disconnectCalls).toBe(1);
|
||||
expect(h.logs.some(l => l.includes('boot did not complete'))).toBe(true);
|
||||
});
|
||||
|
||||
test('a completed boot clears the deadline (no spurious exit)', async () => {
|
||||
const h = makeHarness();
|
||||
h.opts.bootTimeoutMs = 20;
|
||||
await runServe(h.engine as unknown as BrainEngine, [], h.opts);
|
||||
|
||||
// Give the (cleared) deadline window time to fire if the clear failed.
|
||||
await new Promise(r => setTimeout(r, 50));
|
||||
expect(h.engine.disconnectCalls).toBe(0);
|
||||
expect(h.logs.some(l => l.includes('boot did not complete'))).toBe(false);
|
||||
});
|
||||
|
||||
test('bootTimeoutMs = 0 disables the deadline', async () => {
|
||||
const h = makeHarness();
|
||||
let resolveBoot!: () => void;
|
||||
h.opts.startMcpServer = () => new Promise<void>(r => { resolveBoot = r; });
|
||||
h.opts.bootTimeoutMs = 0;
|
||||
const running = runServe(h.engine as unknown as BrainEngine, [], h.opts);
|
||||
|
||||
await new Promise(r => setTimeout(r, 30));
|
||||
expect(h.engine.disconnectCalls).toBe(0);
|
||||
resolveBoot();
|
||||
await running;
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user