Merge pull request #3078 from matrix-org/dbkr/group_call_double_strea…

…m_init Handle group call getting initialised twice in quick succession
matrix-org · Jan 20, 2023 · 4a6e9a0 · 4a6e9a0
2 parents c7210b9 + ea5ce8d
commit 4a6e9a0
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 18 deletions.
diff --git a/src/webrtc/groupCall.ts b/src/webrtc/groupCall.ts
@@ -208,6 +208,7 @@ export class GroupCall extends TypedEventEmitter<
     private resendMemberStateTimer: ReturnType<typeof setInterval> | null = null;
     private initWithAudioMuted = false;
     private initWithVideoMuted = false;
+    private initCallFeedPromise?: Promise<void>;
 
     public constructor(
         private client: MatrixClient,
@@ -347,36 +348,43 @@ export class GroupCall extends TypedEventEmitter<
         );
     }
 
-    public async initLocalCallFeed(): Promise<CallFeed> {
-        logger.log(`groupCall ${this.groupCallId} initLocalCallFeed`);
-
+    public async initLocalCallFeed(): Promise<void> {
         if (this.state !== GroupCallState.LocalCallFeedUninitialized) {
             throw new Error(`Cannot initialize local call feed in the "${this.state}" state.`);
         }
-
         this.state = GroupCallState.InitializingLocalCallFeed;
 
-        let stream: MediaStream;
+        // wraps the real method to serialise calls, because we don't want to try starting
+        // multiple call feeds at once
+        if (this.initCallFeedPromise) return this.initCallFeedPromise;
 
-        let disposed = false;
-        const onState = (state: GroupCallState): void => {
-            if (state === GroupCallState.LocalCallFeedUninitialized) {
-                disposed = true;
-            }
-        };
-        this.on(GroupCallEvent.GroupCallStateChanged, onState);
+        try {
+            this.initCallFeedPromise = this.initLocalCallFeedInternal();
+            await this.initCallFeedPromise;
+        } finally {
+            this.initCallFeedPromise = undefined;
+        }
+    }
+
+    private async initLocalCallFeedInternal(): Promise<void> {
+        logger.log(`groupCall ${this.groupCallId} initLocalCallFeed`);
+
+        let stream: MediaStream;
 
         try {
             stream = await this.client.getMediaHandler().getUserMediaStream(true, this.type === GroupCallType.Video);
         } catch (error) {
             this.state = GroupCallState.LocalCallFeedUninitialized;
             throw error;
-        } finally {
-            this.off(GroupCallEvent.GroupCallStateChanged, onState);
         }
 
-        // The call could've been disposed while we were waiting
-        if (disposed) throw new Error("Group call disposed");
+        // The call could've been disposed while we were waiting, and could
+        // also have been started back up again (hello, React 18) so if we're
+        // still in this 'initializing' state, carry on, otherwise bail.
+        if (this._state !== GroupCallState.InitializingLocalCallFeed) {
+            this.client.getMediaHandler().stopUserMediaStream(stream);
+            throw new Error("Group call disposed while gathering media stream");
+        }
 
         const callFeed = new CallFeed({
             client: this.client,
@@ -396,8 +404,6 @@ export class GroupCall extends TypedEventEmitter<
         this.addUserMediaFeed(callFeed);
 
         this.state = GroupCallState.LocalCallFeedInitialized;
-
-        return callFeed;
     }
 
     public async updateLocalUsermediaStream(stream: MediaStream): Promise<void> {

diff --git a/src/webrtc/mediaHandler.ts b/src/webrtc/mediaHandler.ts
@@ -57,6 +57,9 @@ export class MediaHandler extends TypedEventEmitter<
     public userMediaStreams: MediaStream[] = [];
     public screensharingStreams: MediaStream[] = [];
 
+    // Promise chain to serialise calls to getMediaStream
+    private getMediaStreamPromise?: Promise<MediaStream>;
+
     public constructor(private client: MatrixClient) {
         super();
     }
@@ -196,6 +199,19 @@ export class MediaHandler extends TypedEventEmitter<
      * @returns based on passed parameters
      */
     public async getUserMediaStream(audio: boolean, video: boolean, reusable = true): Promise<MediaStream> {
+        // Serialise calls, othertwise we can't sensibly re-use the stream
+        if (this.getMediaStreamPromise) {
+            this.getMediaStreamPromise = this.getMediaStreamPromise.then(() => {
+                return this.getUserMediaStreamInternal(audio, video, reusable);
+            });
+        } else {
+            this.getMediaStreamPromise = this.getUserMediaStreamInternal(audio, video, reusable);
+        }
+
+        return this.getMediaStreamPromise;
+    }
+
+    private async getUserMediaStreamInternal(audio: boolean, video: boolean, reusable: boolean): Promise<MediaStream> {
         const shouldRequestAudio = audio && (await this.hasAudioDevice());
         const shouldRequestVideo = video && (await this.hasVideoDevice());