From 671ef09948684b5a6f3a935d2e4d804b076302a5 Mon Sep 17 00:00:00 2001 From: Wintermute Date: Tue, 5 May 2026 05:53:48 +0000 Subject: [PATCH] fix: zombie process accumulation + health endpoint timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fixes for cascading failure mode in long-running deployments: 1. cli.ts: Install SIGCHLD handler to reap zombie children. Bun (like Node) only auto-reaps when a handler is registered. Without this, child processes spawned by the worker (embed batches, shell jobs, sub-agents) become zombies when they exit, accumulating in the PID table. 2. serve-http.ts: Add 5s timeout to /health endpoint's getStats() call. When the DB connection pool is saturated (e.g., from zombie processes holding phantom connections), getStats() hangs indefinitely, making the server appear dead to health checks even though it's running. 3. worker.ts: Call engine.disconnect() in the finally block after draining in-flight jobs. Releases PgBouncer connection slots immediately on shutdown rather than waiting for TCP keepalive expiry. 4. supervisor.ts + autopilot.ts: Auto-detect tini on PATH and wrap the spawned worker with it. Belt-and-suspenders with the SIGCHLD handler — tini catches children spawned by native addons that bypass the JS event loop. Zero-config: works when tini is installed, silently skips when not. --- src/cli.ts | 7 +++++++ src/commands/autopilot.ts | 13 +++++++++++-- src/commands/serve-http.ts | 19 ++++++++++++++++--- src/core/minions/supervisor.ts | 31 ++++++++++++++++++++++++++++--- src/core/minions/worker.ts | 4 ++++ 5 files changed, 66 insertions(+), 8 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index d8906a0da..9c1544722 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -1,5 +1,12 @@ #!/usr/bin/env bun +// Reap zombie child processes. Bun (like Node) only auto-reaps children when +// a SIGCHLD handler is installed. Without this, child processes spawned by the +// worker (embed batches, shell jobs, sub-agents) become zombies when they exit, +// accumulating in the PID table and holding phantom DB connection slots. +// A no-op handler is sufficient — the runtime calls waitpid() internally. +process.on('SIGCHLD', () => {}); + import { readFileSync } from 'fs'; import { loadConfig, toEngineConfig } from './core/config.ts'; import type { BrainEngine } from './core/engine.ts'; diff --git a/src/commands/autopilot.ts b/src/commands/autopilot.ts index 7d6b11cb9..11ffc00f1 100644 --- a/src/commands/autopilot.ts +++ b/src/commands/autopilot.ts @@ -162,10 +162,19 @@ export async function runAutopilot(engine: BrainEngine, args: string[]) { // worker. Bare `gbrain jobs work` has no default; the supervisor and // autopilot are the production paths that opt in. const args = ['jobs', 'work', '--max-rss', '2048']; - const child = spawn(cliPath, args, { stdio: 'inherit', env: process.env }); + + // Wrap with tini when available — reaps zombie children from shell jobs + // and embed batches that outlive a watchdog-killed worker. + let tiniPath = ''; + try { tiniPath = execSync('which tini', { encoding: 'utf8', timeout: 2000 }).trim(); } catch {} + const spawnCmd = tiniPath || cliPath; + const spawnArgs = tiniPath ? ['--', cliPath, ...args] : args; + + const child = spawn(spawnCmd, spawnArgs, { stdio: 'inherit', env: process.env }); workerProc = child; lastWorkerStartTime = Date.now(); - console.log(`[autopilot] Minions worker spawned (pid: ${child.pid}, watchdog: 2048MB)`); + console.log(`[autopilot] Minions worker spawned (pid: ${child.pid}, watchdog: 2048MB${tiniPath ? ', tini: active' : ''})`); + child.on('exit', (code) => { workerProc = null; if (stopping) return; diff --git a/src/commands/serve-http.ts b/src/commands/serve-http.ts index 87e7b505b..2bd81d3d8 100644 --- a/src/commands/serve-http.ts +++ b/src/commands/serve-http.ts @@ -226,10 +226,23 @@ export async function runServeHttp(engine: BrainEngine, options: ServeHttpOption // --------------------------------------------------------------------------- app.get('/health', async (_req, res) => { try { - const stats = await engine.getStats(); + // 5s timeout — a slow/saturated DB connection pool must not make the + // entire server look dead to orchestrators and load balancers. + const stats = await Promise.race([ + engine.getStats(), + new Promise((_, reject) => + setTimeout(() => reject(new Error('health_timeout')), 5000), + ), + ]); res.json({ status: 'ok', version: VERSION, engine: config.engine, ...stats }); - } catch { - res.status(503).json({ error: 'service_unavailable', error_description: 'Database connection failed' }); + } catch (e: unknown) { + const msg = e instanceof Error ? e.message : 'unknown'; + res.status(503).json({ + error: 'service_unavailable', + error_description: msg === 'health_timeout' + ? 'Health check timed out (database pool may be saturated)' + : 'Database connection failed', + }); } }); diff --git a/src/core/minions/supervisor.ts b/src/core/minions/supervisor.ts index a36411950..de4b08a6d 100644 --- a/src/core/minions/supervisor.ts +++ b/src/core/minions/supervisor.ts @@ -26,7 +26,7 @@ * 3 PID file unwritable (permission / path error) */ -import { spawn, type ChildProcess } from 'child_process'; +import { spawn, execFileSync, type ChildProcess } from 'child_process'; import { closeSync, existsSync, @@ -138,6 +138,8 @@ export class MinionSupervisor { private child: ChildProcess | null = null; private crashCount = 0; private lastStartTime = 0; + /** Path to tini binary for zombie reaping, or empty string when absent. */ + private readonly tiniPath: string; private stopping = false; private inBackoff = false; private healthInFlight = false; @@ -151,6 +153,16 @@ export class MinionSupervisor { constructor(engine: BrainEngine, opts: Partial & { cliPath: string }) { this.engine = engine; this.opts = { ...DEFAULTS, ...opts }; + + // Detect tini for zombie reaping. Resolved once at construction so we + // don't shell out on every respawn. Belt-and-suspenders with the + // SIGCHLD handler in cli.ts — tini catches children spawned by native + // addons that bypass the JS event loop. + let tini = ''; + try { + tini = execFileSync('which', ['tini'], { encoding: 'utf8', timeout: 2000 }).trim(); + } catch { /* not installed — that's fine */ } + this.tiniPath = tini; } /** @@ -439,9 +451,18 @@ export class MinionSupervisor { this.lastStartTime = Date.now(); + // Wrap with tini when available — reaps zombie children that the + // SIGCHLD handler in cli.ts might miss (native addons, edge cases). + const spawnCmd = this.tiniPath + ? this.tiniPath + : this.opts.cliPath; + const spawnArgs = this.tiniPath + ? ['--', this.opts.cliPath, ...args] + : args; + let child: ChildProcess; try { - child = spawn(this.opts.cliPath, args, { + child = spawn(spawnCmd, spawnArgs, { stdio: 'inherit', env, }); @@ -459,7 +480,11 @@ export class MinionSupervisor { this.child = child; - this.emit('worker_spawned', { pid: child.pid, cli_path: this.opts.cliPath }); + this.emit('worker_spawned', { + pid: child.pid, + cli_path: this.opts.cliPath, + ...(this.tiniPath ? { tini: true } : {}), + }); // Async spawn errors (ENOENT, EACCES after the fork/exec). Node fires // 'error' first, then 'exit' with code=null. We log the error; the diff --git a/src/core/minions/worker.ts b/src/core/minions/worker.ts index 5f800111c..1c814705c 100644 --- a/src/core/minions/worker.ts +++ b/src/core/minions/worker.ts @@ -422,6 +422,10 @@ export class MinionWorker extends EventEmitter { ]); } + // Release database connection pool so PgBouncer slots are freed + // immediately rather than waiting for TCP keepalive timeout. + try { await this.engine.disconnect(); } catch {} + console.log('Minion worker stopped.'); } }