From aa36b84a47ccbd2916bb0061e4b8152ca360fc3d Mon Sep 17 00:00:00 2001 From: Russell Ballestrini Date: Tue, 2 Jun 2026 11:36:05 -0400 Subject: [PATCH] =?UTF-8?q?zebra-spaces:=20stop=20subscribe=E2=86=92close?= =?UTF-8?q?=E2=86=92subscribe=20loop=20=E2=80=94=20listeners=20stay=20conn?= =?UTF-8?q?ected?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of the 'late listener can't see screens' symptom: my recent 'rebuild sub PC on failure' commit (6a72e77) triggered on connection state 'closed' as well as 'failed', AND the guard `if (sfuSubPC === pc)` only worked because we ASSUMED Chrome would fire the state change asynchronously. Chrome fires it SYNCHRONOUSLY during pc.close(), at which point sfuSubPC still points at the closing pc — guard passes, rebuild fires. Then sfuUnsubscribe closes the new PC, which triggers another rebuild. SFU log showed listeners cycling subscribe → 40s → close → subscribe forever. Fix: 1. onconnectionstatechange now only rebuilds on 'failed' (the actually- terminal state). 'closed' = sfuUnsubscribe(), 'disconnected' = transient and WebRTC may recover on its own. 2. sfuUnsubscribe nulls sfuSubPC BEFORE pc.close(), so even if the handler fired synchronously the === guard would correctly fail. 3. visibilitychange handler also tightened to only fire on 'failed' — same reasoning. The rebuild path for actual ICE failures still works (state goes 'connected' → 'disconnected' → 'failed' → rebuild). 83 FSM tests still green. --- web/zebra-spaces.html | 52 ++++++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 20 deletions(-) diff --git a/web/zebra-spaces.html b/web/zebra-spaces.html index 813d2cf..688f280 100644 --- a/web/zebra-spaces.html +++ b/web/zebra-spaces.html @@ -2095,15 +2095,19 @@ async function sfuSubscribe(){ * sfuUnsubscribe + sfuSubscribe. The auto-rejoin code that follows * a hard refresh handles the WS side; this handles the SFU side. */ pc.onconnectionstatechange = () => { - const s = pc.connectionState; - if (s === 'failed' || s === 'closed'){ - logLine('err', 'sfu sub PC ' + s + ' — rebuilding'); - if (sfuSubPC === pc){ - sfuUnsubscribe().then(() => { - if (wantConnected && roomID) sfuSubscribe().catch(e => logLine('err','sfu re-subscribe: '+e.message)); - }); - } - } + /* 'failed' is terminal ICE failure (we should rebuild). + * 'closed' is OUR OWN sfuUnsubscribe() — never rebuild on that + * (Chrome fires the state change synchronously before sfuSubPC is + * null'd, which used to trigger a subscribe→close→subscribe loop + * every time anyone deliberately tore down the sub). + * 'disconnected' is transient — let WebRTC try to recover before + * we yank the rug. */ + if (pc.connectionState !== 'failed') return; + if (sfuSubPC !== pc) return; + logLine('err', 'sfu sub PC failed — rebuilding'); + sfuUnsubscribe().then(() => { + if (wantConnected && roomID) sfuSubscribe().catch(e => logLine('err','sfu re-subscribe: '+e.message)); + }); }; /* SSE: server pushes renegotiation offers when publisher set changes. * We answer each via POST /answer. ping events are keepalive only. @@ -2142,7 +2146,15 @@ async function sfuSubscribe(){ async function sfuUnsubscribe(){ if (sfuSubEvents){ try { sfuSubEvents.close(); } catch(_){} sfuSubEvents = null; } - if (sfuSubPC){ try { sfuSubPC.close(); } catch(_){} sfuSubPC = null; sfuSubPeerID = null; } + /* null out sfuSubPC FIRST, then close — Chrome fires the + * connectionstatechange handler SYNCHRONOUSLY during pc.close(), and + * the handler's `if (sfuSubPC === pc)` guard depends on the global + * already being null. Otherwise the handler thinks the close was a + * 'failed' rebuild trigger and we get a subscribe/close loop. */ + const oldPC = sfuSubPC; + sfuSubPC = null; + sfuSubPeerID = null; + if (oldPC){ try { oldPC.close(); } catch(_){} } sfuStreamsByPubHex.clear(); } @@ -3246,14 +3258,14 @@ refreshMicList(); document.addEventListener('visibilitychange', () => { if (document.visibilityState !== 'visible') return; if (!wantConnected || !sfuSubPC) return; - const s = sfuSubPC.connectionState; - if (s === 'failed' || s === 'closed' || s === 'disconnected'){ - logLine('err', 'visibility back, sub PC ' + s + ' — rebuilding'); - const pc = sfuSubPC; - sfuUnsubscribe().then(() => { - if (wantConnected && roomID) sfuSubscribe().catch(e => logLine('err','sfu re-subscribe: '+e.message)); - }); - } + /* Only rebuild on terminal failure. 'disconnected' is transient + * (WebRTC tries to recover); 'closed' never happens here because + * sfuUnsubscribe() nulls sfuSubPC before pc.close(). */ + if (sfuSubPC.connectionState !== 'failed') return; + logLine('err', 'visibility back, sub PC failed — rebuilding'); + sfuUnsubscribe().then(() => { + if (wantConnected && roomID) sfuSubscribe().catch(e => logLine('err','sfu re-subscribe: '+e.message)); + }); }); /* ================================================================== @@ -3348,8 +3360,8 @@ logLine('', 'ready — pick a handle, type a rendezvous code, enter the space');