Fix some MatrixCall leaks and use a shared AudioContext (#2484)

* Fix some MatrixCall leaks and use a shared AudioContext

These leaks, combined with the dozens of AudioContexts floating around in memory across different CallFeeds, could cause some really bad performance issues and audio crashes on Chrome.

* Fully release the AudioContext in CallFeed's dispose method

* Fix tests

Author: robintown

.eslintrc.js

@@ -55,6 +55,10 @@ module.exports = {
             // We're okay with assertion errors when we ask for them
             "@typescript-eslint/no-non-null-assertion": "off",
 
+            // The base rule produces false positives
+            "func-call-spacing": "off",
+            "@typescript-eslint/func-call-spacing": ["error"],
+
             "quotes": "off",
             // We use a `logger` intermediary module
             "no-console": "error",

spec/unit/webrtc/call.spec.ts

@@ -59,6 +59,17 @@ const DUMMY_SDP = (
     "a=ssrc:3619738545 cname:2RWtmqhXLdoF4sOi\r\n"
 );
 
+class MockMediaStreamAudioSourceNode {
+    connect() {}
+}
+
+class MockAudioContext {
+    constructor() {}
+    createAnalyser() { return {}; }
+    createMediaStreamSource() { return new MockMediaStreamAudioSourceNode(); }
+    close() {}
+}
+
 class MockRTCPeerConnection {
     localDescription: RTCSessionDescription;
 
@@ -162,6 +173,9 @@ describe('Call', function() {
         // @ts-ignore Mock
         global.document = {};
 
+        // @ts-ignore Mock
+        global.AudioContext = MockAudioContext;
+
         client = new TestClient("@alice:foo", "somedevice", "token", undefined, {});
         // We just stub out sendEvent: we're not interested in testing the client's
         // event sending code here

src/webrtc/audioContext.ts

@@ -0,0 +1,44 @@
+/*
+Copyright 2022 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+let audioContext: AudioContext | null = null;
+let refCount = 0;
+
+/**
+ * Acquires a reference to the shared AudioContext.
+ * It's highly recommended to reuse this AudioContext rather than creating your
+ * own, because multiple AudioContexts can be problematic in some browsers.
+ * Make sure to call releaseContext when you're done using it.
+ * @returns {AudioContext} The shared AudioContext
+ */
+export const acquireContext = (): AudioContext => {
+    if (audioContext === null) audioContext = new AudioContext();
+    refCount++;
+    return audioContext;
+};
+
+/**
+ * Signals that one of the references to the shared AudioContext has been
+ * released, allowing the context and associated hardware resources to be
+ * cleaned up if nothing else is using it.
+ */
+export const releaseContext = () => {
+    refCount--;
+    if (refCount === 0) {
+        audioContext.close();
+        audioContext = null;
+    }
+};

src/webrtc/call.ts

@@ -327,6 +327,7 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
     private opponentCaps: CallCapabilities;
     private iceDisconnectedTimeout: ReturnType<typeof setTimeout>;
     private inviteTimeout: ReturnType<typeof setTimeout>;
+    private readonly removeTrackListeners = new Map<MediaStream, () => void>();
 
     // The logic of when & if a call is on hold is nontrivial and explained in is*OnHold
     // This flag represents whether we want the other party to be on hold
@@ -841,18 +842,25 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
         this.setState(CallState.Ringing);
 
         if (event.getLocalAge()) {
-            setTimeout(() => {
-                if (this.state == CallState.Ringing) {
-                    logger.debug(`Call ${this.callId} invite has expired. Hanging up.`);
-                    this.hangupParty = CallParty.Remote; // effectively
-                    this.setState(CallState.Ended);
-                    this.stopAllMedia();
-                    if (this.peerConn.signalingState != 'closed') {
-                        this.peerConn.close();
-                    }
-                    this.emit(CallEvent.Hangup, this);
+            // Time out the call if it's ringing for too long
+            const ringingTimer = setTimeout(() => {
+                logger.debug(`Call ${this.callId} invite has expired. Hanging up.`);
+                this.hangupParty = CallParty.Remote; // effectively
+                this.setState(CallState.Ended);
+                this.stopAllMedia();
+                if (this.peerConn.signalingState != 'closed') {
+                    this.peerConn.close();
                 }
+                this.emit(CallEvent.Hangup, this);
             }, invite.lifetime - event.getLocalAge());
+
+            const onState = (state: CallState) => {
+                if (state !== CallState.Ringing) {
+                    clearTimeout(ringingTimer);
+                    this.off(CallEvent.State, onState);
+                }
+            };
+            this.on(CallEvent.State, onState);
         }
     }
 
@@ -1986,12 +1994,19 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
 
         const stream = ev.streams[0];
         this.pushRemoteFeed(stream);
-        stream.addEventListener("removetrack", () => {
-            if (stream.getTracks().length === 0) {
-                logger.info(`Call ${this.callId} removing track streamId: ${stream.id}`);
-                this.deleteFeedByStream(stream);
-            }
-        });
+
+        if (!this.removeTrackListeners.has(stream)) {
+            const onRemoveTrack = () => {
+                if (stream.getTracks().length === 0) {
+                    logger.info(`Call ${this.callId} removing track streamId: ${stream.id}`);
+                    this.deleteFeedByStream(stream);
+                    stream.removeEventListener("removetrack", onRemoveTrack);
+                    this.removeTrackListeners.delete(stream);
+                }
+            };
+            stream.addEventListener("removetrack", onRemoveTrack);
+            this.removeTrackListeners.set(stream, onRemoveTrack);
+        }
     };
 
     private onDataChannel = (ev: RTCDataChannelEvent): void => {
@@ -2290,6 +2305,11 @@ export class MatrixCall extends TypedEventEmitter<CallEvent, CallEventHandlerMap
             this.callLengthInterval = null;
         }
 
+        for (const [stream, listener] of this.removeTrackListeners) {
+            stream.removeEventListener("removetrack", listener);
+        }
+        this.removeTrackListeners.clear();
+
         this.callStatsAtEnd = await this.collectCallStats();
 
         // Order is important here: first we stopAllMedia() and only then we can deleteAllFeeds()

src/webrtc/callFeed.ts

@@ -15,6 +15,7 @@ limitations under the License.
 */
 
 import { SDPStreamMetadataPurpose } from "./callEventTypes";
+import { acquireContext, releaseContext } from "./audioContext";
 import { MatrixClient } from "../client";
 import { RoomMember } from "../models/room-member";
 import { logger } from "../logger";
@@ -118,10 +119,8 @@ export class CallFeed extends TypedEventEmitter<CallFeedEvent, EventHandlerMap>
     }
 
     private initVolumeMeasuring(): void {
-        const AudioContext = window.AudioContext || window.webkitAudioContext;
-        if (!this.hasAudioTrack || !AudioContext) return;
-
-        this.audioContext = new AudioContext();
+        if (!this.hasAudioTrack) return;
+        if (!this.audioContext) this.audioContext = acquireContext();
 
         this.analyser = this.audioContext.createAnalyser();
         this.analyser.fftSize = 512;
@@ -211,7 +210,7 @@ export class CallFeed extends TypedEventEmitter<CallFeedEvent, EventHandlerMap>
      */
     public measureVolumeActivity(enabled: boolean): void {
         if (enabled) {
-            if (!this.audioContext || !this.analyser || !this.frequencyBinCount || !this.hasAudioTrack) return;
+            if (!this.analyser || !this.frequencyBinCount || !this.hasAudioTrack) return;
 
             this.measuringVolumeActivity = true;
             this.volumeLooper();
@@ -288,5 +287,11 @@ export class CallFeed extends TypedEventEmitter<CallFeedEvent, EventHandlerMap>
 
     public dispose(): void {
         clearTimeout(this.volumeLooperTimeout);
+        this.stream?.removeEventListener("addtrack", this.onAddTrack);
+        if (this.audioContext) {
+            this.audioContext = null;
+            this.analyser = null;
+            releaseContext();
+        }
     }
 }