From 87929bfd0b68058a4f24f2930200a585b2d2d1de Mon Sep 17 00:00:00 2001 From: Waters Church Date: Thu, 17 Sep 2026 16:06:47 -0400 Subject: [PATCH] Don't take the server down when the module unmounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- modules/spl-meter/module.json | 2 +- modules/spl-meter/server.js | 19 +++++++++++++++---- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/modules/spl-meter/module.json b/modules/spl-meter/module.json index e469e98..520978a 100644 --- a/modules/spl-meter/module.json +++ b/modules/spl-meter/module.json @@ -1,7 +1,7 @@ { "id": "spl-meter", "name": "SPL Meter", - "version": "0.3.0", + "version": "0.3.1", "proddash": ">=1.5.0", "description": "Calibrated sound level, Leq and RTA from an audio input on this machine", "permissions": [ diff --git a/modules/spl-meter/server.js b/modules/spl-meter/server.js index d3609a4..98fb47c 100644 --- a/modules/spl-meter/server.js +++ b/modules/spl-meter/server.js @@ -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