From 73f265b11efb88f49aed0169b62b1bc474180bd5 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 2 Jun 2026 22:42:30 -0700 Subject: [PATCH] feat(core): out-of-band hard-deadline watchdog primitive (#1633) Bun eval-Worker that SIGTERM->grace->SIGKILLs its own process from a separate OS thread, so a sync whose main event loop is starved (ReDoS spin) still dies. Signals SELF (no PID-reuse footgun). Empirically validated on Bun 1.3.13. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/core/process-watchdog.ts | 172 +++++++++++++++++++++++++++ test/fixtures/watchdog-harness.ts | 33 +++++ test/process-watchdog.serial.test.ts | 67 +++++++++++ test/process-watchdog.test.ts | 61 ++++++++++ 4 files changed, 333 insertions(+) create mode 100644 src/core/process-watchdog.ts create mode 100644 test/fixtures/watchdog-harness.ts create mode 100644 test/process-watchdog.serial.test.ts create mode 100644 test/process-watchdog.test.ts diff --git a/src/core/process-watchdog.ts b/src/core/process-watchdog.ts new file mode 100644 index 000000000..60b7b5c36 --- /dev/null +++ b/src/core/process-watchdog.ts @@ -0,0 +1,172 @@ +/** + * Out-of-band hard-deadline watchdog (#1633). + * + * THE PROBLEM. A `gbrain sync` that spins (e.g. synchronous catastrophic-regex + * in pack link-inference) STARVES the main event loop. When the loop never + * yields, the SIGTERM handler (process-cleanup.ts) can't run, a `--timeout` + * `setTimeout` can't fire, and the abort-flag checks between import iterations + * can't run either. The process becomes unkillable-by-SIGTERM and, under cron, + * orphans pile up for 24h+ (the reported incident). The ONLY thing that kills a + * loop-starved process is an OS signal delivered from OUTSIDE that loop. + * + * THE MECHANISM. A Bun `worker_threads` Worker runs on a real, independent OS + * thread with its own event loop. Its timer fires even while the main thread is + * in an unyielding synchronous loop. At the deadline it sends SIGTERM to its own + * process (a clean-shutdown chance if the loop happens to be responsive); at + * deadline+grace it sends SIGKILL (uncatchable — guaranteed death even when + * starved). Signaling SELF (`process.kill(process.pid, ...)`) has no PID-reuse + * footgun: the current process's PID is never reused while it's alive. (The + * rejected alternative — a detached child that signals the PARENT pid — CAN hit + * PID reuse and kill an innocent process.) + * + * `eval: true` keeps the worker body an inline string so it bakes into the + * `bun build --compile` binary with no separate-file embedding to worry about. + * Empirically validated on Bun 1.3.13 (a Worker timer fired + SIGKILLed the + * process while main was in `while(true){}`). + * + * ┌─ main thread (may be starved) ──────────────┐ ┌─ watchdog worker (OS thread) ─┐ + * │ sync work / ReDoS spin / connect hang │ │ t=deadline -> SIGTERM │ + * │ ...never yields... │ │ t=deadline+grace-> SIGKILL │ + * │ on clean finish: handle.dispose() ──────────┼──▶│ worker.terminate() │ + * └──────────────────────────────────────────────┘ └────────────────────────────────┘ + * + * Reusable beyond sync (autopilot / cycle are follow-up adopters): the API is + * just (deadline, grace, label). + */ + +import { Worker } from 'node:worker_threads'; + +export type WatchdogAction = 'wait' | 'sigterm' | 'sigkill'; + +/** + * Pure decision function — the watchdog's whole state machine, extracted so it's + * unit-testable without spawning threads or real timers. + * elapsed < deadline -> 'wait' + * deadline <= elapsed < +grace -> 'sigterm' (clean-shutdown chance) + * elapsed >= deadline + grace -> 'sigkill' (guaranteed) + */ +export function watchdogDecision(elapsedMs: number, deadlineMs: number, graceMs: number): WatchdogAction { + if (elapsedMs >= deadlineMs + graceMs) return 'sigkill'; + if (elapsedMs >= deadlineMs) return 'sigterm'; + return 'wait'; +} + +export interface ProcessWatchdogOpts { + /** Wall-clock ms after which SIGTERM is sent. Must be > 0 or the watchdog is a no-op. */ + deadlineMs: number; + /** ms after the deadline before SIGKILL. Default 30_000. */ + graceMs?: number; + /** Prefix for stderr log lines, e.g. 'sync-watchdog'. Default 'watchdog'. */ + label?: string; + /** Periodic "still alive, kill in Ns" heartbeat interval ms. 0 = off (default). */ + heartbeatMs?: number; + /** Injectable warn sink (tests). Default writes to process.stderr. */ + onWarn?: (msg: string) => void; +} + +export interface WatchdogHandle { + /** Tear down the watchdog (clean completion). Idempotent. */ + dispose(): void; + /** True when an out-of-band worker is actually running (false on no-op / fallback). */ + readonly active: boolean; +} + +const DEFAULT_GRACE_MS = 30_000; + +function defaultWarn(msg: string): void { + try { process.stderr.write(msg + '\n'); } catch { /* stderr may be broken */ } +} + +const INERT: WatchdogHandle = { dispose() {}, get active() { return false; } }; + +/** + * Worker body (runs on its own OS thread). Inline string so `eval: true` bakes + * it into the compiled binary. Uses only built-ins available in a Bun worker. + * + * `label` is validated by the caller to a safe charset before it reaches here, + * so it can't break the string literal or inject log lines. + */ +const WORKER_SRC = ` +const { workerData } = require('node:worker_threads'); +const { deadlineMs, graceMs, label, heartbeatMs } = workerData; +const t0 = Date.now(); +function w(m) { try { process.stderr.write('[' + label + '] ' + m + '\\n'); } catch (e) {} } +if (heartbeatMs > 0) { + const hb = setInterval(() => { + const elapsed = Math.round((Date.now() - t0) / 1000); + const killIn = Math.round((deadlineMs + graceMs - (Date.now() - t0)) / 1000); + w('parent alive ' + elapsed + 's elapsed, hard-kill in ~' + killIn + 's'); + }, heartbeatMs); + if (typeof hb.unref === 'function') hb.unref(); +} +setTimeout(() => { + w('deadline reached (' + Math.round(deadlineMs/1000) + 's) — sending SIGTERM for graceful shutdown'); + try { process.kill(process.pid, 'SIGTERM'); } catch (e) {} +}, deadlineMs); +setTimeout(() => { + w('grace expired — sending SIGKILL (event loop was starved; this is the orphan-pileup backstop)'); + try { process.kill(process.pid, 'SIGKILL'); } catch (e) {} +}, deadlineMs + graceMs); +`; + +/** + * Install the out-of-band hard-deadline watchdog. Returns a handle whose + * `dispose()` MUST be called on clean completion (a `finally`) so the worker is + * torn down. If the deadline is non-positive, returns an inert no-op handle. + * + * Fallback: if the Worker can't be constructed (unexpected on Bun), degrades to + * an in-process timer with a loud warning. The in-process timer canNOT fire + * under event-loop starvation — it only covers the responsive case — so the + * warning tells the operator the hard guarantee is degraded. + */ +export function installProcessWatchdog(opts: ProcessWatchdogOpts): WatchdogHandle { + const warn = opts.onWarn ?? defaultWarn; + const deadlineMs = Math.floor(opts.deadlineMs); + const graceMs = Math.max(0, Math.floor(opts.graceMs ?? DEFAULT_GRACE_MS)); + // Sanitize label to a safe charset (defends the inline worker string + log lines). + const label = (opts.label ?? 'watchdog').replace(/[^A-Za-z0-9_.:-]/g, '').slice(0, 40) || 'watchdog'; + const heartbeatMs = Math.max(0, Math.floor(opts.heartbeatMs ?? 0)); + + if (!Number.isFinite(deadlineMs) || deadlineMs <= 0) return INERT; + + try { + const worker = new Worker(WORKER_SRC, { + eval: true, + workerData: { deadlineMs, graceMs, label, heartbeatMs }, + }); + // Don't let the watchdog keep the process alive past clean completion. + (worker as unknown as { unref?: () => void }).unref?.(); + // A worker-side error must never crash the host; log and move on. + worker.on('error', (err) => warn(`[${label}] watchdog worker error: ${err instanceof Error ? err.message : String(err)}`)); + let disposed = false; + return { + dispose() { + if (disposed) return; + disposed = true; + void worker.terminate(); + }, + get active() { return !disposed; }, + }; + } catch (err) { + // Fallback: in-process timer. Starvation-vulnerable — say so loudly. + warn( + `[${label}] could not start out-of-band watchdog (${err instanceof Error ? err.message : String(err)}); ` + + `falling back to an in-process timer that will NOT fire if the event loop is starved.`, + ); + let killed = false; + const term = setTimeout(() => { try { process.kill(process.pid, 'SIGTERM'); } catch { /* */ } }, deadlineMs); + const kill = setTimeout(() => { killed = true; try { process.kill(process.pid, 'SIGKILL'); } catch { /* */ } }, deadlineMs + graceMs); + (term as unknown as { unref?: () => void }).unref?.(); + (kill as unknown as { unref?: () => void }).unref?.(); + let disposed = false; + return { + dispose() { + if (disposed || killed) return; + disposed = true; + clearTimeout(term); + clearTimeout(kill); + }, + get active() { return !disposed; }, + }; + } +} diff --git a/test/fixtures/watchdog-harness.ts b/test/fixtures/watchdog-harness.ts new file mode 100644 index 000000000..a0ca5bb32 --- /dev/null +++ b/test/fixtures/watchdog-harness.ts @@ -0,0 +1,33 @@ +/** + * Fixture for test/process-watchdog.serial.test.ts. Spawned via `bun`. + * + * Usage: bun watchdog-harness.ts + * starve-with — install the watchdog, then starve the event loop forever. + * The watchdog must SIGKILL this process by deadline+grace. + * starve-without — no watchdog, just starve. Proves the busy loop truly hangs + * (the test kills it). Isolates the watchdog as cause of death. + * clean-dispose — install with a long deadline, dispose immediately, exit 0. + * The watchdog must NOT kill a cleanly-disposed process. + * + * Safety net: the busy loop self-exits after 8s so a failed test kill can't hang CI. + */ +import { installProcessWatchdog } from '../../src/core/process-watchdog.ts'; + +const mode = process.argv[2] ?? 'starve-with'; +const deadlineMs = Number(process.argv[3] ?? 300); +const graceMs = Number(process.argv[4] ?? 150); + +if (mode === 'starve-with' || mode === 'clean-dispose') { + const handle = installProcessWatchdog({ deadlineMs, graceMs, label: 'test-wd' }); + if (mode === 'clean-dispose') { + handle.dispose(); + process.stdout.write('DISPOSED\n'); + process.exit(0); + } +} + +// Starve the main event loop with a synchronous busy loop (simulates ReDoS). +const start = Date.now(); +while (Date.now() - start < 8000) { /* spin — no await, no yield */ } +process.stdout.write('SURVIVED\n'); // must NOT print under starve-with +process.exit(0); diff --git a/test/process-watchdog.serial.test.ts b/test/process-watchdog.serial.test.ts new file mode 100644 index 000000000..2c6e8849b --- /dev/null +++ b/test/process-watchdog.serial.test.ts @@ -0,0 +1,67 @@ +/** + * Bun-pinned integration for the out-of-band watchdog (#1633, plan A4). + * + * Bun's worker_threads Worker is flagged "experimental", and the whole #1633 + * fix rests on a worker timer firing + SIGKILLing the process while the MAIN + * thread is starved by a synchronous loop. These tests spawn a real harness + * process that starves its own loop and assert the watchdog kills it anyway. + * + * Serial because they use real subprocesses + wall-clock timing. + */ +import { describe, test, expect } from 'bun:test'; +import { join } from 'node:path'; + +const HARNESS = join(import.meta.dir, 'fixtures', 'watchdog-harness.ts'); + +async function runHarness( + mode: string, + deadlineMs: number, + graceMs: number, + hardCapMs: number, +): Promise<{ exitCode: number | null; signalled: boolean; elapsedMs: number; stdout: string; killedByTest: boolean }> { + const proc = Bun.spawn(['bun', HARNESS, mode, String(deadlineMs), String(graceMs)], { + stdout: 'pipe', + stderr: 'pipe', + }); + const start = Date.now(); + let killedByTest = false; + const cap = setTimeout(() => { killedByTest = true; proc.kill('SIGKILL'); }, hardCapMs); + await proc.exited; + clearTimeout(cap); + const elapsedMs = Date.now() - start; + const stdout = await new Response(proc.stdout).text(); + // Bun surfaces signal death via exitCode === null + signalCode, or a negative + // exitCode on some platforms. Treat "not a clean 0" as signalled for our purpose. + const signalled = proc.exitCode !== 0; + return { exitCode: proc.exitCode, signalled, elapsedMs, stdout, killedByTest }; +} + +describe('process-watchdog integration (Bun-pinned)', () => { + test('starved process IS killed by the watchdog around deadline+grace', async () => { + // deadline 300 + grace 200 = ~500ms expected death. Hard cap 4s: if the + // watchdog failed, the test's own SIGKILL fires and the assertion catches it. + const r = await runHarness('starve-with', 300, 200, 4000); + expect(r.stdout).not.toContain('SURVIVED'); // the bug symptom + expect(r.killedByTest).toBe(false); // watchdog, not the test, killed it + expect(r.signalled).toBe(true); + // Died well before the harness's 8s self-exit safety net, near deadline+grace. + expect(r.elapsedMs).toBeLessThan(3000); + }, 15000); + + test('control: a starved process WITHOUT the watchdog does not self-exit', async () => { + // Proves the busy loop genuinely starves (so the death above is the watchdog). + // No watchdog installed; the test's hard cap (1.2s) is what kills it. + const r = await runHarness('starve-without', 300, 200, 1200); + expect(r.killedByTest).toBe(true); // only the test's SIGKILL stopped it + expect(r.stdout).not.toContain('SURVIVED'); + }, 15000); + + test('clean dispose: a disposed watchdog never kills the process', async () => { + // Long deadline, disposed immediately, process exits 0 fast and prints DISPOSED. + const r = await runHarness('clean-dispose', 60000, 60000, 5000); + expect(r.exitCode).toBe(0); + expect(r.killedByTest).toBe(false); + expect(r.stdout).toContain('DISPOSED'); + expect(r.elapsedMs).toBeLessThan(4000); + }, 15000); +}); diff --git a/test/process-watchdog.test.ts b/test/process-watchdog.test.ts new file mode 100644 index 000000000..380ae6d33 --- /dev/null +++ b/test/process-watchdog.test.ts @@ -0,0 +1,61 @@ +/** + * Pure-function coverage for the watchdog state machine (#1633). No threads, no + * real timers — the spawn-based integration lives in + * test/process-watchdog.serial.test.ts (Bun-pinned, real processes). + */ +import { describe, test, expect } from 'bun:test'; +import { watchdogDecision, installProcessWatchdog } from '../src/core/process-watchdog.ts'; + +describe('watchdogDecision', () => { + const deadline = 1000; + const grace = 300; + + test('waits before the deadline', () => { + expect(watchdogDecision(0, deadline, grace)).toBe('wait'); + expect(watchdogDecision(999, deadline, grace)).toBe('wait'); + }); + + test('SIGTERM at the deadline boundary (inclusive)', () => { + expect(watchdogDecision(1000, deadline, grace)).toBe('sigterm'); + expect(watchdogDecision(1299, deadline, grace)).toBe('sigterm'); + }); + + test('SIGKILL at deadline+grace boundary (inclusive)', () => { + expect(watchdogDecision(1300, deadline, grace)).toBe('sigkill'); + expect(watchdogDecision(5000, deadline, grace)).toBe('sigkill'); + }); + + test('zero grace goes straight to SIGKILL at the deadline', () => { + expect(watchdogDecision(999, deadline, 0)).toBe('wait'); + expect(watchdogDecision(1000, deadline, 0)).toBe('sigkill'); + }); +}); + +describe('installProcessWatchdog (handle contract)', () => { + test('non-positive deadline returns an inert no-op handle', () => { + const warns: string[] = []; + const h0 = installProcessWatchdog({ deadlineMs: 0, onWarn: (m) => warns.push(m) }); + expect(h0.active).toBe(false); + h0.dispose(); // idempotent, no throw + const hNeg = installProcessWatchdog({ deadlineMs: -5, onWarn: (m) => warns.push(m) }); + expect(hNeg.active).toBe(false); + }); + + test('active handle disposes idempotently without killing the test process', () => { + // Long deadline so it never fires during the test; dispose tears it down. + const h = installProcessWatchdog({ deadlineMs: 60_000, graceMs: 60_000, label: 'unit-wd' }); + expect(h.active).toBe(true); + h.dispose(); + expect(h.active).toBe(false); + h.dispose(); // second dispose is a no-op + expect(h.active).toBe(false); + }); + + test('label is sanitized to a safe charset', () => { + // A nasty label must not throw at construction (it is stripped before the + // inline worker string). We dispose immediately so nothing fires. + const h = installProcessWatchdog({ deadlineMs: 60_000, label: "evil'; \n process.exit(1) //" }); + expect(h.active).toBe(true); + h.dispose(); + }); +});