Merge pull request #203 from thecodingmachine/fixmulitplelisteners

Fixing memory leak with listeners

front/src/Phaser/Game/GameScene.ts

@@ -190,6 +190,7 @@ export class GameScene extends Phaser.Scene {
                 console.log('Player disconnected from server. Reloading scene.');
 
                 this.simplePeer.closeAllConnections();
+                this.simplePeer.unregister();
 
                 const key = 'somekey'+Math.round(Math.random()*10000);
                 const game : Phaser.Scene = GameScene.createFromUrl(this.MapUrlFile, this.instance, key);
@@ -610,6 +611,7 @@ export class GameScene extends Phaser.Scene {
         if(nextSceneKey){
             // We are completely destroying the current scene to avoid using a half-backed instance when coming back to the same map.
             this.connection.closeConnection();
+            this.simplePeer.unregister();
             this.scene.stop();
             this.scene.remove(this.scene.key);
             this.scene.start(nextSceneKey.key, {

front/src/WebRtc/MediaManager.ts

@@ -3,7 +3,10 @@ const videoConstraint: boolean|MediaTrackConstraints = {
     height: { ideal: 720 },
     facingMode: "user"
 };
-export class MediaManager {
+
+type UpdatedLocalStreamCallback = (media: MediaStream) => void;
+
+class MediaManager {
     localStream: MediaStream|null = null;
     private remoteVideo: Map<string, HTMLVideoElement> = new Map<string, HTMLVideoElement>();
     myCamVideo: HTMLVideoElement;
@@ -16,11 +19,9 @@ export class MediaManager {
         audio: true,
         video: videoConstraint
     };
-    updatedLocalStreamCallBack : (media: MediaStream) => void;
+    updatedLocalStreamCallBacks : Set<UpdatedLocalStreamCallback> = new Set<UpdatedLocalStreamCallback>();
-
-    constructor(updatedLocalStreamCallBack : (media: MediaStream) => void) {
-        this.updatedLocalStreamCallBack = updatedLocalStreamCallBack;
 
+    constructor() {
         this.myCamVideo = this.getElementByIdOrFail<HTMLVideoElement>('myCamVideo');
         this.webrtcInAudio = this.getElementByIdOrFail<HTMLAudioElement>('audio-webrtc-in');
         this.webrtcInAudio.volume = 0.2;
@@ -54,6 +55,21 @@ export class MediaManager {
         });
     }
 
+    onUpdateLocalStream(callback: UpdatedLocalStreamCallback): void {
+
+        this.updatedLocalStreamCallBacks.add(callback);
+    }
+
+    removeUpdateLocalStreamEventListener(callback: UpdatedLocalStreamCallback): void {
+        this.updatedLocalStreamCallBacks.delete(callback);
+    }
+
+    private triggerUpdatedLocalStreamCallbacks(stream: MediaStream): void {
+        for (const callback of this.updatedLocalStreamCallBacks) {
+            callback(stream);
+        }
+    }
+
     activeVisio(){
         const webRtc = this.getElementByIdOrFail('webRtc');
         webRtc.classList.add('active');
@@ -64,7 +80,7 @@ export class MediaManager {
         this.cinema.style.display = "block";
         this.constraintsMedia.video = videoConstraint;
         this.getCamera().then((stream: MediaStream) => {
-            this.updatedLocalStreamCallBack(stream);
+            this.triggerUpdatedLocalStreamCallbacks(stream);
         });
     }
 
@@ -79,7 +95,7 @@ export class MediaManager {
             });
         }
         this.getCamera().then((stream) => {
-            this.updatedLocalStreamCallBack(stream);
+            this.triggerUpdatedLocalStreamCallbacks(stream);
         });
     }
 
@@ -88,7 +104,7 @@ export class MediaManager {
         this.microphone.style.display = "block";
         this.constraintsMedia.audio = true;
         this.getCamera().then((stream) => {
-            this.updatedLocalStreamCallBack(stream);
+            this.triggerUpdatedLocalStreamCallbacks(stream);
         });
     }
 
@@ -102,7 +118,7 @@ export class MediaManager {
             });
         }
         this.getCamera().then((stream) => {
-            this.updatedLocalStreamCallBack(stream);
+            this.triggerUpdatedLocalStreamCallbacks(stream);
         });
     }
 
@@ -308,3 +324,5 @@ export class MediaManager {
     }
 
 }
+
+export const mediaManager = new MediaManager();

front/src/WebRtc/SimplePeer.ts

@@ -4,7 +4,7 @@ import {
     WebRtcSignalMessageInterface,
     WebRtcStartMessageInterface
 } from "../Connection";
-import {MediaManager} from "./MediaManager";
+import { mediaManager } from "./MediaManager";
 import * as SimplePeerNamespace from "simple-peer";
 const Peer: SimplePeerNamespace.SimplePeer = require('simple-peer');
 
@@ -22,16 +22,15 @@ export class SimplePeer {
     private WebRtcRoomId: string;
     private Users: Array<UserSimplePeer> = new Array<UserSimplePeer>();
 
-    private MediaManager: MediaManager;
-
     private PeerConnectionArray: Map<string, SimplePeerNamespace.Instance> = new Map<string, SimplePeerNamespace.Instance>();
+    private readonly updateLocalStreamCallback: (media: MediaStream) => void;
 
     constructor(Connection: Connection, WebRtcRoomId: string = "test-webrtc") {
         this.Connection = Connection;
         this.WebRtcRoomId = WebRtcRoomId;
-        this.MediaManager = new MediaManager((stream : MediaStream) => {
+        // We need to go through this weird bound function pointer in order to be able to "free" this reference later.
-            this.updatedLocalStream();
+        this.updateLocalStreamCallback = this.updatedLocalStream.bind(this);
-        });
+        mediaManager.onUpdateLocalStream(this.updateLocalStreamCallback);
         this.initialise();
     }
 
@@ -45,8 +44,8 @@ export class SimplePeer {
             this.receiveWebrtcSignal(message);
         });
 
-        this.MediaManager.activeVisio();
+        mediaManager.activeVisio();
-        this.MediaManager.getCamera().then(() => {
+        mediaManager.getCamera().then(() => {
 
             //receive message start
             this.Connection.receiveWebrtcStart((message: WebRtcStartMessageInterface) => {
@@ -105,8 +104,8 @@ export class SimplePeer {
                 name = userSearch.name;
             }
         }
-        this.MediaManager.removeActiveVideo(user.userId);
+        mediaManager.removeActiveVideo(user.userId);
-        this.MediaManager.addActiveVideo(user.userId, name);
+        mediaManager.addActiveVideo(user.userId, name);
 
         const peer : SimplePeerNamespace.Instance = new Peer({
             initiator: user.initiator ? user.initiator : false,
@@ -143,15 +142,15 @@ export class SimplePeer {
                 }
             });
             if(microphoneActive){
-                this.MediaManager.enabledMicrophoneByUserId(user.userId);
+                mediaManager.enabledMicrophoneByUserId(user.userId);
             }else{
-                this.MediaManager.disabledMicrophoneByUserId(user.userId);
+                mediaManager.disabledMicrophoneByUserId(user.userId);
             }
 
             if(videoActive){
-                this.MediaManager.enabledVideoByUserId(user.userId);
+                mediaManager.enabledVideoByUserId(user.userId);
             }else{
-                this.MediaManager.disabledVideoByUserId(user.userId);
+                mediaManager.disabledVideoByUserId(user.userId);
             }
             this.stream(user.userId, stream);
         });
@@ -167,11 +166,11 @@ export class SimplePeer {
         // eslint-disable-next-line @typescript-eslint/no-explicit-any
         peer.on('error', (err: any) => {
             console.error(`error => ${user.userId} => ${err.code}`, err);
-            this.MediaManager.isError(user.userId);
+            mediaManager.isError(user.userId);
         });
 
         peer.on('connect', () => {
-            this.MediaManager.isConnected(user.userId);
+            mediaManager.isConnected(user.userId);
             console.info(`connect => ${user.userId}`);
         });
 
@@ -192,7 +191,7 @@ export class SimplePeer {
      */
     private closeConnection(userId : string) {
         try {
-            this.MediaManager.removeActiveVideo(userId);
+            mediaManager.removeActiveVideo(userId);
             const peer = this.PeerConnectionArray.get(userId);
             if (peer === undefined) {
                 console.warn("Tried to close connection for user "+userId+" but could not find user")
@@ -215,6 +214,13 @@ export class SimplePeer {
         }
     }
 
+    /**
+     * Unregisters any held event handler.
+     */
+    public unregister() {
+        mediaManager.removeUpdateLocalStreamEventListener(this.updateLocalStreamCallback);
+    }
+
     /**
      *
      * @param userId
@@ -253,11 +259,11 @@ export class SimplePeer {
      */
     private stream(userId : string, stream: MediaStream) {
         if(!stream){
-            this.MediaManager.disabledVideoByUserId(userId);
+            mediaManager.disabledVideoByUserId(userId);
-            this.MediaManager.disabledMicrophoneByUserId(userId);
+            mediaManager.disabledMicrophoneByUserId(userId);
             return;
         }
-        this.MediaManager.addStreamRemoteVideo(userId, stream);
+        mediaManager.addStreamRemoteVideo(userId, stream);
     }
 
     /**
@@ -266,7 +272,7 @@ export class SimplePeer {
      */
     private addMedia (userId : string) {
         try {
-            const localStream: MediaStream|null = this.MediaManager.localStream;
+            const localStream: MediaStream|null = mediaManager.localStream;
             const peer = this.PeerConnectionArray.get(userId);
             if(localStream === null) {
                 //send fake signal