From 390b495252e43c9894d4c014547e746c34179ae7 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 12 Sep 2022 14:41:21 +0100 Subject: [PATCH 1/3] 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. --- src/webrtc/groupCall.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/webrtc/groupCall.ts b/src/webrtc/groupCall.ts index 104eacae0c4..10e8a90ea18 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.log( + `Placing call to ${member.userId}/${opponentDevice.device_id} session ID ${opponentDevice.session_id}.`, + ); try { await newCall.placeCallWithCallFeeds( @@ -887,12 +897,6 @@ export class GroupCall extends TypedEventEmitter< 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 { From 96800bdd6c682ff6496d4a2cf95bfc84a0b8771d Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 12 Sep 2022 14:43:58 +0100 Subject: [PATCH 2/3] Switch to logger.debug --- src/webrtc/groupCall.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/webrtc/groupCall.ts b/src/webrtc/groupCall.ts index 10e8a90ea18..2c4d6f6f3b4 100644 --- a/src/webrtc/groupCall.ts +++ b/src/webrtc/groupCall.ts @@ -869,7 +869,7 @@ export class GroupCall extends TypedEventEmitter< const requestScreenshareFeed = opponentDevice.feeds.some( (feed) => feed.purpose === SDPStreamMetadataPurpose.Screenshare); - logger.log( + logger.debug( `Placing call to ${member.userId}/${opponentDevice.device_id} session ID ${opponentDevice.session_id}.`, ); From ad0914959a759e7a63c3956fe0b12d8571d526cc Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 13 Sep 2022 11:27:09 +0100 Subject: [PATCH 3/3] Fix unit tests --- spec/unit/webrtc/groupCall.spec.ts | 2 ++ src/webrtc/groupCall.ts | 1 + 2 files changed, 3 insertions(+) diff --git a/spec/unit/webrtc/groupCall.spec.ts b/spec/unit/webrtc/groupCall.spec.ts index 74563bda0ec..724778ce624 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 2c4d6f6f3b4..b96ebed7c51 100644 --- a/src/webrtc/groupCall.ts +++ b/src/webrtc/groupCall.ts @@ -891,6 +891,7 @@ export class GroupCall extends TypedEventEmitter< ), ); } + this.removeCall(newCall, CallErrorCode.SignallingFailed); return; }