crash-log SIGTERM/SIGHUP handler never exits — process survives the signal, node --watch hangs on every restart

[joemckie]

Summary

enableCrashLog installs SIGTERM and SIGHUP listeners that write a crash log and then return without terminating. Registering any listener for those signals suppresses Node's default terminate action, so the process survives the signal entirely.

Since enableDevTools turns crash logging on by default, any app using devtools becomes immune to SIGTERM. Under node --watch this means every file save hangs on Waiting for graceful termination... until Node gives up and SIGKILLs, and each restart leaves two storm-crash-*.json files in the cwd.

Reproduction

npm install @orchetron/storm@0.2.0 react@19

// child.tsx
import { render, enableDevTools, Box, Text } from "@orchetron/storm";
import React from "react";

const app = render(<Box><Text>hello</Text></Box>);
enableDevTools(app);
await app.waitUntilExit();

Run it, then kill -TERM <pid>. The process writes two crash logs and keeps running. Same under node --watch child.tsx — touch the file and it hangs on restart.

For contrast, enableDevTools(app, { crashLog: false }) exits immediately, which isolates it to the crash-log handler.

Mechanism

Screen registers its signal handlers first (during render), enableCrashLog second. On SIGTERM:

1. Screen.onSignal runs — cleans up, removes its own listeners, then re-raises via process.kill(process.pid, sig).
2. crash-log.onSignal runs (same dispatch) — writes log feat: storm reconsiler major optimization and upgrade #1, returns without exiting.
3. The re-raised SIGTERM is delivered. Screen's listener is gone, but crash-log's is still installed, so the default action is still suppressed — log fix: update ci and package #2 is written, and again nothing exits.
4. The process runs forever.

That accounts for both symptoms: the duplicate logs (1 ms apart) and the hang.

Suggested fix

src/devtools/crash-log.ts — uninstall and re-raise so the default handler runs:

let signalHandled = false;
const onSignal = (sig) => {
    if (!installed || signalHandled) return;
    // Screen.onSignal re-raises after its own cleanup, which would otherwise
    // land here a second time and write a duplicate log.
    signalHandled = true;

    const data = buildCrashData(app, profiler, frames, includeTree, { signal: sig });
    const path = writeCrashLog(dir, data);
    if (path) {
        try { process.stderr.write(`[storm] Crash log written: ${path}\n`); } catch {}
    }

    // Registering ANY listener suppresses Node's default terminate action —
    // without this the process survives the signal.
    process.removeListener("SIGTERM", onSignal);
    process.removeListener("SIGHUP", onSignal);
    process.kill(process.pid, sig);
};

Verified via pnpm patch against dist/: node --watch restarts cleanly, and SIGINT / SIGTERM / SIGHUP all terminate in 12–18 ms (previously SIGTERM and SIGHUP never terminated at all).

Two related things you may want to consider

1. Screen.onSignal's re-raise assumes it is the only listener. It removes its own handlers and re-raises, which only terminates if nothing else is listening. Any user code that registers a SIGTERM listener — a DB pool draining, a telemetry flush — reintroduces exactly this hang, and it'll look like a storm bug. Re-raising is the right pattern, but it might be worth documenting that storm-managed shutdown expects to own the signal, or having Screen explicitly process.exit() after a grace period rather than relying on the default action.

2. Should SIGTERM produce a crash log at all? SIGTERM is an ordinary "please shut down" — it's what node --watch, kill, systemd, Docker and CI all send on normal stop. Treating it as a crash means a routine restart writes a 60-frame profiler dump to the cwd every time. In a --watch loop that's a file per save. SIGSEGV-style genuine crashes and uncaughtException seem worth capturing; a clean SIGTERM maybe less so, or at least opt-in separately from enableDevTools' defaults.

Both are judgement calls rather than bugs, so I've left them alone in my patch — happy to follow whatever you prefer if you'd like a PR.

Environment

• @orchetron/storm 0.2.0 (latest on npm)
• react 19.2.8, react-reconciler 0.31.0
• Node 24.15.0, Linux

Unrelated to #21 / #22 / #23 — this one reproduces on a clean unpatched install with no reconciler involvement.