Don't take the server down when the module unmounts

Deploying 0.3.0 to AB crashed the ProdDash server on restart:

  TypeError: Cannot read properties of null (reading 'streams')
      at server.js:273  req.on('close', () => state.streams.delete(res))

teardown() nulls `state` and then ends every open stream, which fires each
request's close handler — and the handler reached back through the `state`
it had just discarded. Any re-init with a browser connected would do it, so
saving admin config while a dashboard was open was enough to stop the whole
server, every module with it.

The stream handler now closes over this mount's Set rather than the module's
state: after teardown it deletes from a set nobody reads, which is harmless.
The same hazard existed anywhere a callback could outlive the mount — the
keepalive ping, a frame arriving from the listener mid-teardown, and the
/measure interval — and each is closed over or guarded now.

Reproduced first (open an SSE client, save config, watch it fall over) and
confirmed fixed against the same sequence.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Waters Church
2026-09-17 16:06:47 -04:00
co-authored by Claude Opus 5
parent 46d44d494b
commit 87929bfd0b
2 changed files with 16 additions and 5 deletions
+15 -4
View File
@@ -132,6 +132,7 @@ module.exports = {
moduleDir,
log,
onUpdate(update) {
if (!state) return; // a frame in flight when the module unmounted
if (update.error) {
if (update.error !== state.error) log(update.error);
state.error = update.error;
@@ -146,9 +147,10 @@ module.exports = {
},
});
const mineStreams = state.streams;
state.pinger = setInterval(() => {
for (const res of state.streams) {
try { res.write(': ping\n\n'); } catch { state.streams.delete(res); }
for (const res of mineStreams) {
try { res.write(': ping\n\n'); } catch { mineStreams.delete(res); }
}
}, PING_MS);
@@ -260,8 +262,14 @@ module.exports = {
try {
res.write(`event: state\ndata: ${JSON.stringify(snapshot())}\n\n`);
} catch { return; }
state.streams.add(res);
req.on('close', () => state.streams.delete(res));
// Hold this mount's set, not `state`. teardown() nulls `state` and
// then ends every stream, which fires these close handlers — a
// handler that reached through `state` threw on null and took the
// whole server down with it, on any re-init with a browser
// connected. Deleting from a set nobody reads any more is harmless.
const streams = state.streams;
streams.add(res);
req.on('close', () => streams.delete(res));
},
/** Device list for the admin page. Runs the capture tool's --list,
@@ -311,6 +319,7 @@ module.exports = {
const samples = [];
const started = Date.now();
const tick = setInterval(() => {
if (!state) { clearInterval(tick); return; } // module unmounted mid-measure
const ch = state.frame && state.frame.ch.find((c) => c.i === wanted);
if (ch && Number.isFinite(ch.dbfs) && ch.dbfs > -200) samples.push(ch.dbfs);
if (Date.now() - started < MEASURE_MS) return;
@@ -343,6 +352,7 @@ module.exports = {
/* ── helpers ────────────────────────────────────────────────────────── */
function snapshot() {
if (!state) return { error: 'not measuring', calibrated: false, channels: [], frame: null };
return {
error: state.error,
calibrated: state.calibrated,
@@ -356,6 +366,7 @@ function snapshot() {
}
function broadcast() {
if (!state) return; // a listener callback landing after teardown
const payload = snapshot();
const line = JSON.stringify(payload);
if (line === state.lastSent) return; // nothing moved since last frame