From a3e04cecc3dd479c79d76b86463bc85ef6e06c40 Mon Sep 17 00:00:00 2001 From: Valere Date: Fri, 5 Dec 2025 17:22:48 +0100 Subject: [PATCH] add test reproducing the race --- .../localMember/Publisher.test.ts | 129 +++++++++++++++++- 1 file changed, 123 insertions(+), 6 deletions(-) diff --git a/src/state/CallViewModel/localMember/Publisher.test.ts b/src/state/CallViewModel/localMember/Publisher.test.ts index 9b3e5b2a..384ecff0 100644 --- a/src/state/CallViewModel/localMember/Publisher.test.ts +++ b/src/state/CallViewModel/localMember/Publisher.test.ts @@ -14,13 +14,19 @@ import { type Mock, vi, } from "vitest"; -import { ConnectionState as LivekitConenctionState } from "livekit-client"; -import { type BehaviorSubject } from "rxjs"; +import { + ConnectionState as LivekitConenctionState, + LocalParticipant, + type LocalTrack, + type LocalTrackPublication, +} from "livekit-client"; +import { BehaviorSubject } from "rxjs"; import { logger } from "matrix-js-sdk/lib/logger"; import { ObservableScope } from "../../ObservableScope"; import { constant } from "../../Behavior"; import { + flushPromises, mockLivekitRoom, mockLocalParticipant, mockMediaDevices, @@ -33,8 +39,15 @@ import { import { type MuteStates } from "../../MuteStates"; import { FailToStartLivekitConnection } from "../../../utils/errors"; +let scope: ObservableScope; + +beforeEach(() => { + scope = new ObservableScope(); +}); + +afterEach(() => scope.end()); + describe("Publisher", () => { - let scope: ObservableScope; let connection: Connection; let muteStates: MuteStates; beforeEach(() => { @@ -50,7 +63,6 @@ describe("Publisher", () => { setHandler: vi.fn(), }, } as unknown as MuteStates; - scope = new ObservableScope(); connection = { state$: constant({ state: "ConnectedToLkRoom", @@ -62,8 +74,6 @@ describe("Publisher", () => { } as unknown as Connection; }); - afterEach(() => scope.end()); - it("throws if livekit room could not publish", async () => { const publisher = new Publisher( scope, @@ -138,3 +148,110 @@ describe("Publisher", () => { ).toHaveBeenCalledTimes(3); }); }); + + +describe("Bug fix", () => { + + // There is a race condition when creating and publishing tracks while the mute state changes. + // This race condition could cause tracks to be published even though they are muted at the + // beginning of a call coming from lobby. + // This is caused by our stack using manually the low level API to create and publish tracks, + // but also using the higher level setMicrophoneEnabled and setCameraEnabled functions that also create + // and publish tracks, and managing pending publications. + // Race is as follow, on creation of the Publisher we create the tracks then publish them. + // If in the middle of that process the mute state changes: + // - the `setMicrophoneEnabled` will be no-op because it is not aware of our created track and can't see any pending publication + // - If start publication is requested it will publish the track even though there was a mute request. + it.fails("wrongly publish tracks while muted", async () => { + const audioEnabled$ = new BehaviorSubject(true); + const muteStates = { + audio: { + enabled$: audioEnabled$, + unsetHandler: vi.fn(), + setHandler: vi.fn(), + }, + video: { + enabled$: constant(false), + unsetHandler: vi.fn(), + setHandler: vi.fn(), + }, + } as unknown as MuteStates; + + const mockSendDataPacket = vi.fn(); + const mockEngine = { + client: { + sendUpdateLocalMetadata: vi.fn(), + }, + on: vi.fn().mockReturnThis(), + sendDataPacket: mockSendDataPacket, + }; + + // cont mockRoomOptions = {} as InternalRoomOptions; + + const localParticipant = new LocalParticipant( + "local-sid", + "local-identity", + // @ts-expect-error - for that test we want a real LocalParticipant to have the pending publications logic + mockEngine, + {}, + new Map(), + {}, + ); + + const connection = { + state$: constant({ + state: "ConnectedToLkRoom", + livekitConnectionState$: constant(LivekitConenctionState.Connected), + }), + livekitRoom: mockLivekitRoom({ + localParticipant, + }), + } as unknown as Connection; + + const mediaDevices = mockMediaDevices({}); + + const mockTrack = vi.mocked({ + kind: "audio", + mute: vi.fn(), + } as Partial as LocalTrack); + const createTrackLock = Promise.withResolvers(); + const createTrackSpy = vi.spyOn(localParticipant, "createTracks"); + createTrackSpy.mockImplementation(async () => { + await createTrackLock.promise; + return [mockTrack]; + }); + + const publishTrackSpy = vi.spyOn(localParticipant, "publishTrack"); + publishTrackSpy.mockResolvedValue({} as unknown as LocalTrackPublication); + + const publisher = new Publisher( + scope, + connection, + mediaDevices, + muteStates, + constant({ supported: false, processor: undefined }), + logger, + ); + + // Initially the audio is unmuted, so creating tracks should publish the audio track + const createTracks = publisher.createAndSetupTracks(); + publisher.tracks$.subscribe(() => { + void publisher.startPublishing(); + }); + // now mute the audio before allowing track creation to complete + audioEnabled$.next(false); + // const publishing = publisher.startPublishing(); + createTrackLock.resolve(); + await createTracks; + // await publishing; + + await flushPromises(); + + // It should not publish or instead call track.mute() + try { + expect(publishTrackSpy).not.toHaveBeenCalled(); + } catch { + expect(mockTrack.mute).toHaveBeenCalled(); + } + }); +});