From 5e8cdadb2935ab7d1e64b74e39298d4e58a92464 Mon Sep 17 00:00:00 2001 From: Russell Ballestrini Date: Sat, 6 Jun 2026 14:45:39 -0400 Subject: [PATCH] zebra-spaces: send 'bye' on pagehide for listeners even on bfcache (kill closed-firefox roster zombies) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fox 2026-06-06: "closed firefox on both phones [they] are both still in the list" — listener-roster entries persisting indefinitely after mobile Firefox close. Two facts collided: 1. SERVER (proxy.unturf.com main.go aliveJanitorTick ~line 1872): listeners are fully exempt from the heartbeat-stall reaper. Justified fox 2026-06-04 because the page's {type:"alive"} timer throttles hard on backgrounded mobile tabs (1Hz on Android, paused on iOS power-save) — without the exemption, mobile listeners lost their seat every time they tab-switched. 2. CLIENT (this file, sendByeIfRealClose): pagehide with event.persisted=true means the page is going into bfcache (mobile app-switch / tab-close-to-bfcache / lock screen), so we SKIPPED the 'bye' message to keep PCs warm for resume. Justified fox 2026-06-03 because "the phone leaving and coming back cannot hear the music" — bye-driven SFU eviction killed the speaker's publish + subscribe PCs. Net: a mobile Firefox close fires pagehide(persisted=true) → no 'bye' → server has only readTimeout (120s) + hiccup grace (8s) to detect the dead socket → ~128s of phantom listener seat in every other client's roster. The bfcache justification on the CLIENT side was always specific to speakers (they have PCs to protect). LISTENERS: - have no publisher PC - finalizeLeave at main.go:1089 explicitly exempts them from evictFromSFU (their subscriber PC stays alive through the bye) - re-handshake fresh on pageshow via POST /subscribe (same path as a cold join) So for listeners, sending 'bye' on persisted=true is roster-only cleanup: peer-left broadcast, members.delete(uuid) on every other client, brief disappearance from the room. On pageshow they re-handshake and reappear — same UX as a cold rejoin, which already works. Fix: split the bfcache rule by role. Send 'bye' on pagehide even when persisted=true if myRole === 'listener'. Speakers keep the original bfcache skip exactly. Server-side backstop (60s listener-specific TTL replacing the full exemption) ships as a separate commit in proxy.unturf.com so 'bye' losses (carrier NAT eating the TCP shutdown, abrupt process kill, custom Firefox close paths) still get reaped within the minute. Pinned by test/sendbye-fsm.test.js — extracts sendByeIfRealClose from this page and drives 9 scenarios covering each (role, persisted) combination plus defensive edges (no event, ws not open, post-demote listener state). Wired into test-all via test-sendbye target. --- Makefile | 11 ++- test/sendbye-fsm.test.js | 141 +++++++++++++++++++++++++++++++++++++++ web/zebra-spaces.html | 41 ++++++++---- 3 files changed, 178 insertions(+), 15 deletions(-) create mode 100644 test/sendbye-fsm.test.js diff --git a/Makefile b/Makefile index fe33665..d4949ed 100644 --- a/Makefile +++ b/Makefile @@ -72,6 +72,13 @@ test-self-listener: test-listener-audio: @node test/listener-audio-attach.test.js +# sendByeIfRealClose role-aware bfcache split — pins that listeners +# send bye even on pagehide(persisted=true) so closed-Firefox phones +# don't sit as 128s roster zombies, while speakers keep the bfcache +# skip so their PCs survive a tab-switch nap. +test-sendbye: + @node test/sendbye-fsm.test.js + # zebra-spaces JS↔Go protocol parity + vault + ed25519 + (optionally) a live # server flow. The live-server tier auto-runs when proxy.unturf.com sits # alongside this checkout AND has a Go toolchain — we build the relay binary @@ -89,7 +96,7 @@ test-zebra-spaces: ZEBRA_SPACES_BINARY=$$bin node test/zebra-spaces.test.js; \ rc=$$?; rm -f /tmp/zspc-signal-test; exit $$rc -test-all: test/unit test/integration test/functional test-web test-fsm test-video-removal test-mesh test-mod-actions test-self-listener test-listener-audio test-zebra-spaces +test-all: test/unit test/integration test/functional test-web test-fsm test-video-removal test-mesh test-mod-actions test-self-listener test-listener-audio test-sendbye test-zebra-spaces @echo "--- unit ---" @./test/unit @echo "--- integration ---" @@ -110,6 +117,8 @@ test-all: test/unit test/integration test/functional test-web test-fsm test-vide @node test/self-listener-fsm.test.js @echo "--- listener audio attach ---" @node test/listener-audio-attach.test.js + @echo "--- sendBye FSM ---" + @node test/sendbye-fsm.test.js @echo "--- zebra-spaces ---" @$(MAKE) -s test-zebra-spaces diff --git a/test/sendbye-fsm.test.js b/test/sendbye-fsm.test.js new file mode 100644 index 0000000..49ea2c9 --- /dev/null +++ b/test/sendbye-fsm.test.js @@ -0,0 +1,141 @@ +#!/usr/bin/env node +/* sendByeIfRealClose state-machine tests. + * + * node test/sendbye-fsm.test.js + * + * Pins the listener / speaker split on pagehide(persisted=true): + * + * - speaker + persisted=true -> no bye (bfcache nap; keep PCs warm) + * - speaker + persisted=false -> bye (real unload) + * - listener + persisted=true -> bye (no PCs to protect; roster + * cleanup matters more than + * bfcache continuity) + * - listener + persisted=false -> bye (real unload) + * + * Background: fox 2026-06-03 hit "phone leaving and coming back cannot + * hear the music" when bye was always sent — the SFU eviction killed + * the speaker's PCs and the resumed page couldn't recover them. The + * fix was to skip bye on persisted=true. fox 2026-06-06 hit the + * opposite: "closed firefox on both phones [they] are both still in + * the list" because listeners are exempt from the server's + * aliveJanitor and the bfcache skip left them as 120s+8s zombies. The + * split below threads the needle. The function is extracted from + * web/zebra-spaces.html at test time so the spec can't drift. */ + +const fs = require('fs'); +const path = require('path'); +const src = fs.readFileSync(path.join(__dirname, '..', 'web', 'zebra-spaces.html'), 'utf8'); + +function extractFn(re){ + const m = src.match(re); + if (!m) throw new Error('could not find ' + re); + let i = src.indexOf('{', m.index + m[0].length), depth = 0, j = i; + for (; j < src.length; j++){ + if (src[j] === '{') depth++; + else if (src[j] === '}') { if (--depth === 0) { j++; break; } } + } + return src.slice(m.index, j); +} + +const sendByeSrc = extractFn(/function sendByeIfRealClose\(/); + +/* Build a sandbox where myRole + ws are mutable so each scenario can + * configure them, and ws.send is a spy that records every call. */ +function makeHarness(initialRole){ + const sent = []; + const ws = { + readyState: 1, /* OPEN — WebSocket.OPEN === 1 in browsers */ + send(msg){ sent.push(msg); }, + }; + const factory = new Function( + 'wsRef', 'WebSocket', 'initialRole', + 'let myRole = initialRole;\n' + + 'const ws = wsRef;\n' + + sendByeSrc + '\n' + + 'return {\n' + + ' setRole(r){ myRole = r; },\n' + + ' setWsState(s){ ws.readyState = s; },\n' + + ' sendByeIfRealClose,\n' + + '};\n' + ); + return { sent, ws, api: factory(ws, { OPEN: 1 }, initialRole) }; +} + +let pass = 0, fail = 0; +function test(name, fn){ + try { fn(); console.log(' ✓ ' + name); pass++; } + catch (e){ console.log(' ✗ ' + name + ' — ' + (e.message || e)); fail++; } +} +function eq(a, b, msg){ + if (a !== b) throw new Error((msg || 'expected') + ' — got ' + JSON.stringify(a) + ' want ' + JSON.stringify(b)); +} + +console.log('sendByeIfRealClose:'); + +test('speaker + persisted=true (bfcache): bye NOT sent (PCs must survive the nap)', () => { + const h = makeHarness('speaker'); + h.api.sendByeIfRealClose({ persisted: true }); + eq(h.sent.length, 0, 'no bye on bfcache'); +}); + +test('speaker + persisted=false (real unload): bye sent', () => { + const h = makeHarness('speaker'); + h.api.sendByeIfRealClose({ persisted: false }); + eq(h.sent.length, 1, 'bye sent'); + eq(JSON.parse(h.sent[0]).type, 'bye'); +}); + +test('host + persisted=true (bfcache): bye NOT sent (host is a speaker role)', () => { + const h = makeHarness('host'); + h.api.sendByeIfRealClose({ persisted: true }); + eq(h.sent.length, 0); +}); + +test('cohost + persisted=true (bfcache): bye NOT sent', () => { + const h = makeHarness('cohost'); + h.api.sendByeIfRealClose({ persisted: true }); + eq(h.sent.length, 0); +}); + +test('listener + persisted=true (bfcache): bye SENT (no PCs to protect, roster cleanup wins)', () => { + /* Pins the fox 2026-06-06 fix: closed Firefox on phone leaves listener + * zombies because aliveJanitor exempts listeners + bfcache skip blocked + * bye. Listeners ride opposite the speaker bfcache rule. */ + const h = makeHarness('listener'); + h.api.sendByeIfRealClose({ persisted: true }); + eq(h.sent.length, 1, 'bye sent on bfcache for listener'); + eq(JSON.parse(h.sent[0]).type, 'bye'); +}); + +test('listener + persisted=false (real unload): bye sent', () => { + const h = makeHarness('listener'); + h.api.sendByeIfRealClose({ persisted: false }); + eq(h.sent.length, 1); +}); + +test('any role + no event (defensive): bye sent', () => { + /* Some browsers call event handlers with undefined. Don't crash. */ + const h = makeHarness('speaker'); + h.api.sendByeIfRealClose(undefined); + eq(h.sent.length, 1, 'bye sent when event missing'); +}); + +test('ws not open: no bye attempted (no throw, no send)', () => { + const h = makeHarness('listener'); + h.api.setWsState(0); /* CONNECTING */ + h.api.sendByeIfRealClose({ persisted: true }); + eq(h.sent.length, 0, 'no send while ws not open'); +}); + +test('role flips to listener mid-session, then bfcache fires: bye sent', () => { + /* a user promoted/demoted during the session — the role-at-pagehide + * is what matters, not the role-at-join. */ + const h = makeHarness('speaker'); + h.api.setRole('listener'); + h.api.sendByeIfRealClose({ persisted: true }); + eq(h.sent.length, 1, 'bye sent under post-demote listener state'); +}); + +console.log(''); +console.log((fail === 0 ? 'PASS' : 'FAIL') + ' — ' + pass + ' passed, ' + fail + ' failed'); +process.exit(fail === 0 ? 0 : 1); diff --git a/web/zebra-spaces.html b/web/zebra-spaces.html index e603d84..bf86062 100644 --- a/web/zebra-spaces.html +++ b/web/zebra-spaces.html @@ -7571,20 +7571,33 @@ refreshSpeakerList(); * this, a closed tab dies silently → hiccup grace → 8s of trailing * audio. Fox 2026-06-03. * - * Critical guard: pagehide ALSO fires when the page goes into the - * back-forward cache (mobile app-switch / lock screen / minimise), - * with event.persisted=true. We must NOT send bye in that case — - * the page is still alive, just paused; on pageshow it resumes with - * the same WS / SFU / mesh state. Sending bye here would force the - * server to evict the SFU PCs, and when the phone comes back the - * resumed audio path stays muted (Fox 2026-06-03: "now the phone - * leaving and coming back cannot hear the music"). + * Critical guard for SPEAKERS: pagehide ALSO fires when the page + * goes into the back-forward cache (mobile app-switch / lock screen + * / minimise), with event.persisted=true. A speaker on bfcache + * MUST NOT send bye — finalizeLeave would post evictFromSFU on + * their publisher AND subscriber PCs, and when the phone comes + * back the resumed audio path stays muted (Fox 2026-06-03: "now + * the phone leaving and coming back cannot hear the music"). * - * The JS heartbeat handles the bfcache case independently — while - * the page is bfcached, setInterval is paused, so the server's - * aliveTTL fires after 45s if the user doesn't come back. */ + * LISTENERS take the opposite branch: send bye even on persisted. + * finalizeLeave already exempts listeners from evictFromSFU + * (signal-server main.go line ~1089), so a listener bye is roster- + * only — peer-left broadcast, member entry deleted, no eviction. + * On pageshow the listener re-handshakes (sub PC + WS join) like + * a cold join, same path that already works. The cost is a brief + * "rejoin" appearance in other clients' rosters; the benefit is + * the immediate disappearance of closed-Firefox-on-phone zombies + * who otherwise wait out the 120s readTimeout + 8s hiccup grace + * because listeners are exempt from the server's aliveJanitor. + * Fox 2026-06-06: "closed firefox on both phones [they] are both + * still in the list". + * + * The JS heartbeat handles the speaker bfcache case independently + * — while the page is bfcached, setInterval is paused, so the + * server's aliveTTL fires after 45s if the user doesn't come back. */ function sendByeIfRealClose(ev){ - if (ev && ev.persisted) return; // bfcache — page is napping, not dying + /* speaker on bfcache: keep the PCs warm, ride out the nap */ + if (ev && ev.persisted && myRole !== 'listener') return; try { if (ws && ws.readyState === WebSocket.OPEN){ ws.send(JSON.stringify({ type: 'bye' })); @@ -7891,8 +7904,8 @@ logLine('', 'ready — pick a handle, type a rendezvous code, enter the space');