From 41cee6f1cc29c14f60bfcde709ec322aeafbebb3 Mon Sep 17 00:00:00 2001
From: David Baker <dbkr@users.noreply.github.com>
Date: Tue, 13 Sep 2022 16:30:34 +0100
Subject: [PATCH] Fix race in creating calls (#2662)

* Fix race in creating calls

We ran an async function between checking for an existing call and
adding the new one to the map, so it would have been possible to
start creating another call while we were placing the first call.
This changes the code to add the call to the map as soon as we've
created it.

Also adds more logging.

* Switch to logger.debug

* Fix unit tests
---
 spec/unit/webrtc/groupCall.spec.ts |  2 ++
 src/webrtc/groupCall.ts            | 19 ++++++++++++-------
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/spec/unit/webrtc/groupCall.spec.ts b/spec/unit/webrtc/groupCall.spec.ts
index 74563bda0..724778ce6 100644
--- a/spec/unit/webrtc/groupCall.spec.ts
+++ b/spec/unit/webrtc/groupCall.spec.ts
@@ -533,6 +533,7 @@ describe('Group Call', function() {
                 let newCall: MatrixCall;
                 while (
                     (newCall = groupCall1.getCallByUserId(client2.userId)) === undefined ||
+                    newCall.peerConn === undefined ||
                     newCall.callId == oldCall.callId
                 ) {
                     await flushPromises();
@@ -643,6 +644,7 @@ describe('Group Call', function() {
                 groupCall.localCallFeed.setAudioVideoMuted = jest.fn();
                 const setAVMutedArray = groupCall.calls.map(call => {
                     call.localUsermediaFeed.setAudioVideoMuted = jest.fn();
+                    call.localUsermediaFeed.isVideoMuted = jest.fn().mockReturnValue(true);
                     return call.localUsermediaFeed.setAudioVideoMuted;
                 });
                 const tracksArray = groupCall.calls.reduce((acc, call) => {
diff --git a/src/webrtc/groupCall.ts b/src/webrtc/groupCall.ts
index 104eacae0..b96ebed7c 100644
--- a/src/webrtc/groupCall.ts
+++ b/src/webrtc/groupCall.ts
@@ -856,12 +856,22 @@ export class GroupCall extends TypedEventEmitter<
             },
         );
 
+        if (existingCall) {
+            logger.debug(`Replacing call ${existingCall.callId} to ${member.userId} with ${newCall.callId}`);
+            this.replaceCall(existingCall, newCall, CallErrorCode.NewSession);
+        } else {
+            logger.debug(`Adding call ${newCall.callId} to ${member.userId}`);
+            this.addCall(newCall);
+        }
+
         newCall.isPtt = this.isPtt;
 
         const requestScreenshareFeed = opponentDevice.feeds.some(
             (feed) => feed.purpose === SDPStreamMetadataPurpose.Screenshare);
 
-        logger.log(`Placing call to ${member.userId}.`);
+        logger.debug(
+            `Placing call to ${member.userId}/${opponentDevice.device_id} session ID ${opponentDevice.session_id}.`,
+        );
 
         try {
             await newCall.placeCallWithCallFeeds(
@@ -881,18 +891,13 @@ export class GroupCall extends TypedEventEmitter<
                     ),
                 );
             }
+            this.removeCall(newCall, CallErrorCode.SignallingFailed);
             return;
         }
 
         if (this.dataChannelsEnabled) {
             newCall.createDataChannel("datachannel", this.dataChannelOptions);
         }
-
-        if (existingCall) {
-            this.replaceCall(existingCall, newCall, CallErrorCode.NewSession);
-        } else {
-            this.addCall(newCall);
-        }
     };
 
     public getDeviceForMember(userId: string): IGroupCallRoomMemberDevice {