Fixing memory leak with listeners

The listeners from MediaManager and SimplePeer were never removed, leading to a huge amount of listeners all over the applications when switching regularly of scene.
This commit is contained in:
David Négrier 2020-06-23 14:56:57 +02:00
parent 12ce0e16ed
commit d78006e106
3 changed files with 56 additions and 30 deletions

View File

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

View File

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

View File

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