From 3634c78d7148378ad3bdfff6073c64a1148ee0bc Mon Sep 17 00:00:00 2001 From: Santhosh Vaiyapuri Date: Thu, 9 Apr 2026 12:53:37 +0200 Subject: [PATCH] fix: ignore late ICE candidates after cleanup for RN speech detector --- .../client/src/helpers/RNSpeechDetector.ts | 54 ++++++++++++++----- .../__tests__/RNSpeechDetector.test.ts | 52 ++++++++++++++++++ 2 files changed, 92 insertions(+), 14 deletions(-) create mode 100644 packages/client/src/helpers/__tests__/RNSpeechDetector.test.ts diff --git a/packages/client/src/helpers/RNSpeechDetector.ts b/packages/client/src/helpers/RNSpeechDetector.ts index 492dca7eb8..7902ec548c 100644 --- a/packages/client/src/helpers/RNSpeechDetector.ts +++ b/packages/client/src/helpers/RNSpeechDetector.ts @@ -3,10 +3,11 @@ import { SoundStateChangeHandler } from './sound-detector'; import { videoLoggerSystem } from '../logger'; export class RNSpeechDetector { - private pc1 = new RTCPeerConnection({}); - private pc2 = new RTCPeerConnection({}); + private readonly pc1 = new RTCPeerConnection({}); + private readonly pc2 = new RTCPeerConnection({}); private audioStream: MediaStream | undefined; private externalAudioStream: MediaStream | undefined; + private isStopped = false; constructor(externalAudioStream?: MediaStream) { this.externalAudioStream = externalAudioStream; @@ -17,30 +18,31 @@ export class RNSpeechDetector { */ public async start(onSoundDetectedStateChanged: SoundStateChangeHandler) { try { + this.isStopped = false; const audioStream = this.externalAudioStream != null ? this.externalAudioStream : await navigator.mediaDevices.getUserMedia({ audio: true }); this.audioStream = audioStream; - this.pc1.addEventListener('icecandidate', (e) => { - this.pc2.addIceCandidate(e.candidate).catch(() => { - // do nothing - }); - }); - this.pc2.addEventListener('icecandidate', async (e) => { - this.pc1.addIceCandidate(e.candidate).catch(() => { - // do nothing - }); - }); - this.pc2.addEventListener('track', (e) => { + const onPc1IceCandidate = (e: RTCPeerConnectionIceEvent) => { + this.forwardIceCandidate(this.pc2, e.candidate); + }; + const onPc2IceCandidate = (e: RTCPeerConnectionIceEvent) => { + this.forwardIceCandidate(this.pc1, e.candidate); + }; + const onTrackPc2 = (e: RTCTrackEvent) => { e.streams[0].getTracks().forEach((track) => { // In RN, the remote track is automatically added to the audio output device // so we need to mute it to avoid hearing the audio back // @ts-expect-error _setVolume is a private method in react-native-webrtc track._setVolume(0); }); - }); + }; + + this.pc1.addEventListener('icecandidate', onPc1IceCandidate); + this.pc2.addEventListener('icecandidate', onPc2IceCandidate); + this.pc2.addEventListener('track', onTrackPc2); audioStream .getTracks() @@ -55,6 +57,9 @@ export class RNSpeechDetector { onSoundDetectedStateChanged, ); return () => { + this.pc1.removeEventListener('icecandidate', onPc1IceCandidate); + this.pc2.removeEventListener('icecandidate', onPc2IceCandidate); + this.pc2.removeEventListener('track', onTrackPc2); unsubscribe(); this.stop(); }; @@ -69,6 +74,9 @@ export class RNSpeechDetector { * Stops the speech detection and releases all allocated resources. */ private stop() { + if (this.isStopped) return; + this.isStopped = true; + this.pc1.close(); this.pc2.close(); @@ -185,4 +193,22 @@ export class RNSpeechDetector { this.audioStream.release(); } } + + private forwardIceCandidate( + destination: RTCPeerConnection, + candidate: RTCIceCandidate | null, + ) { + if ( + this.isStopped || + !candidate || + destination.signalingState === 'closed' + ) { + return; + } + destination.addIceCandidate(candidate).catch(() => { + // silently ignore the error + const logger = videoLoggerSystem.getLogger('RNSpeechDetector'); + logger.info('cannot add ice candidate - ignoring'); + }); + } } diff --git a/packages/client/src/helpers/__tests__/RNSpeechDetector.test.ts b/packages/client/src/helpers/__tests__/RNSpeechDetector.test.ts new file mode 100644 index 0000000000..4aec55d4da --- /dev/null +++ b/packages/client/src/helpers/__tests__/RNSpeechDetector.test.ts @@ -0,0 +1,52 @@ +import '../../rtc/__tests__/mocks/webrtc.mocks'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { RNSpeechDetector } from '../RNSpeechDetector'; + +describe('RNSpeechDetector', () => { + // Shared test setup stubs RTCPeerConnection with a vi.fn constructor. + // We keep a typed handle to that constructor to inspect created instances. + let rtcPeerConnectionMockCtor: ReturnType; + + beforeEach(() => { + rtcPeerConnectionMockCtor = + globalThis.RTCPeerConnection as unknown as ReturnType; + rtcPeerConnectionMockCtor.mockClear(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('ignores late ICE candidates after cleanup', async () => { + const stream = { + getTracks: () => [], + } as unknown as MediaStream; + const detector = new RNSpeechDetector(stream); + + const cleanup = await detector.start(() => {}); + cleanup(); + + // start() creates two peer connections (pc1 and pc2). We pull them from + // constructor call results to inspect listener wiring and ICE forwarding. + const [pc1, pc2] = rtcPeerConnectionMockCtor.mock.results.map( + (result) => result.value, + ); + + // Find the registered ICE callback and invoke it manually after cleanup to + // simulate a late ICE event arriving during teardown. + const onIceCandidate = pc1.addEventListener.mock.calls.find( + ([eventName]: [string]) => eventName === 'icecandidate', + )?.[1] as ((e: RTCPeerConnectionIceEvent) => void) | undefined; + + expect(onIceCandidate).toBeDefined(); + onIceCandidate?.({ + candidate: { candidate: 'candidate:1 1 UDP 0 127.0.0.1 11111 typ host' }, + } as unknown as RTCPeerConnectionIceEvent); + + expect(pc1.removeEventListener).toHaveBeenCalledWith( + 'icecandidate', + onIceCandidate, + ); + expect(pc2.addIceCandidate).not.toHaveBeenCalled(); + }); +});