From 9dd693ef34d47a887c74b7c81389b201ad9f1a4d Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Tue, 23 Jun 2026 08:11:20 +0000 Subject: [PATCH] fix(desktop): guard startHosting re-entry so screen share survives reconnect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 0.8.3 connect-retry collided with the auto-start-hosting effect, which re-fires startHosting whenever isHosting is false — which it stays through the retries. With no re-entrancy guard, overlapping startHosting calls each built their own Room and connect/disconnected on top of each other (a DUPLICATE_IDENTITY / CLIENT_REQUEST_LEAVE storm in the SFU logs). The screen share got published on a connection that was then torn down, so the host saw camera + black presentation. - startHosting now bails if a start is already in-flight or a room exists. - On failure it tears down roomRef so the next (sequential) attempt is clean. - A terminal Disconnected releases roomRef so re-hosting still works. Tests: concurrent start builds one connection; re-host works after a terminal disconnect; existing retry/reconnect cases still pass (7/7). Co-Authored-By: Claude Opus 4.8 --- .../hooks/useWebRTCHostSFUAPI.test.ts | 46 +++++++++++++++++++ .../src/renderer/hooks/useWebRTCHostSFUAPI.ts | 23 ++++++++++ 2 files changed, 69 insertions(+) diff --git a/apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.test.ts b/apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.test.ts index 8899e79..232a333 100644 --- a/apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.test.ts +++ b/apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.test.ts @@ -121,6 +121,10 @@ const mockGetUserMedia = vi.fn(); describe('useWebRTCHostSFUAPI', () => { beforeEach(() => { vi.clearAllMocks(); + // clearAllMocks resets call history but not implementations; a prior test's + // mockRejectedValue would otherwise leak into the next. Reset connect/disconnect. + mockConnect.mockReset().mockResolvedValue(undefined); + mockDisconnect.mockReset().mockResolvedValue(undefined); mockRemoteParticipants.clear(); mockTrackPublications.clear(); @@ -304,4 +308,46 @@ describe('useWebRTCHostSFUAPI', () => { expect(result.current.error).toBe('could not establish pc connection'); vi.useRealTimers(); }); + + it('ignores a concurrent startHosting call (no duplicate connection storm)', async () => { + const { result } = renderHook(() => + useWebRTCHostSFUAPI({ sessionId: 'session-1', hostId: 'host-1', localStream: null }) + ); + + // Two overlapping starts (as the auto-start effect can do) — the second + // must bail on the re-entrancy guard rather than build a second Room. + await act(async () => { + await Promise.all([result.current.startHosting(), result.current.startHosting()]); + await Promise.resolve(); + }); + + expect(mockConnect).toHaveBeenCalledTimes(1); + expect(result.current.isHosting).toBe(true); + }); + + it('can re-host after a terminal disconnect (guard released)', async () => { + const { result } = renderHook(() => + useWebRTCHostSFUAPI({ sessionId: 'session-1', hostId: 'host-1', localStream: null }) + ); + + await act(async () => { + await result.current.startHosting(); + await Promise.resolve(); + }); + expect(mockConnect).toHaveBeenCalledTimes(1); + + // Terminal disconnect — must release the room ref. + act(() => { + mockRoomInstance.emit('connectionStateChanged', 'disconnected'); + }); + expect(result.current.isHosting).toBe(false); + + // A fresh start now proceeds instead of bailing on the (stale) guard. + await act(async () => { + await result.current.startHosting(); + await Promise.resolve(); + }); + expect(mockConnect).toHaveBeenCalledTimes(2); + expect(result.current.isHosting).toBe(true); + }); }); diff --git a/apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.ts b/apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.ts index 63411b9..73182aa 100644 --- a/apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.ts +++ b/apps/desktop/src/renderer/hooks/useWebRTCHostSFUAPI.ts @@ -99,6 +99,7 @@ export function useWebRTCHostSFUAPI({ const [hostMicStream, setHostMicStream] = useState(null); const roomRef = useRef(null); + const startingRef = useRef(false); const viewersRef = useRef>(new Map()); const authTokenRef = useRef(null); const hostMicStreamRef = useRef(null); @@ -219,6 +220,15 @@ export function useWebRTCHostSFUAPI({ // Start hosting (sets up LiveKit room and voice -- screen sharing is optional) const startHosting = useCallback(async () => { + // Re-entrancy guard. The auto-start effect re-fires startHosting whenever + // isHosting is false — which stays false through the connect retries below. + // Without this, overlapping startHosting calls each build their own Room and + // connect/disconnect on top of each other (a DUPLICATE_IDENTITY / + // CLIENT_REQUEST_LEAVE storm), leaving the screen share published on a + // connection that then gets torn down → camera shows but presentation is + // black. One attempt at a time; the effect retries sequentially after. + if (startingRef.current || roomRef.current) return; + startingRef.current = true; try { // Capture host microphone before connecting try { @@ -354,6 +364,9 @@ export function useWebRTCHostSFUAPI({ // own reconnect attempts (transient blips emit Reconnecting instead). setIsHosting(false); setError('Disconnected from server'); + // Release the dead room so the auto-start effect can re-host cleanly + // (the re-entrancy guard keys off roomRef). + roomRef.current = null; } else if (state === LKConnectionState.Connected) { // Back to healthy — drop any error left over from a reconnect. setError(null); @@ -416,6 +429,16 @@ export function useWebRTCHostSFUAPI({ } catch (err) { console.error('[WebRTCHostSFUAPI] Failed to start hosting:', err); setError(err instanceof Error ? err.message : 'Failed to start hosting'); + // Tear the failed room down so the next (sequential) attempt starts clean + // rather than bailing on the roomRef guard forever. + try { + await roomRef.current?.disconnect(); + } catch { + // already gone — ignore + } + roomRef.current = null; + } finally { + startingRef.current = false; } }, [sessionId, hostId, addViewer, removeViewer, handleDataReceived]);