From b2e28a81cfdfb3d764cc0abd27ad3e45f50c9647 Mon Sep 17 00:00:00 2001 From: pabloFuente Date: Sun, 26 Apr 2020 22:22:39 +0200 Subject: [PATCH] openvidu-browser: fix 'on' and 'once' event methods memory leak --- .../src/OpenVidu/EventDispatcher.ts | 101 ++++++++++++++++++ openvidu-browser/src/OpenVidu/Publisher.ts | 2 +- openvidu-browser/src/OpenVidu/Session.ts | 33 ++---- openvidu-browser/src/OpenVidu/Stream.ts | 36 ++----- .../src/OpenVidu/StreamManager.ts | 41 +++---- .../Interfaces/Public/EventDispatcher.ts | 43 -------- openvidu-browser/src/index.ts | 2 +- 7 files changed, 129 insertions(+), 129 deletions(-) create mode 100644 openvidu-browser/src/OpenVidu/EventDispatcher.ts delete mode 100644 openvidu-browser/src/OpenViduInternal/Interfaces/Public/EventDispatcher.ts diff --git a/openvidu-browser/src/OpenVidu/EventDispatcher.ts b/openvidu-browser/src/OpenVidu/EventDispatcher.ts new file mode 100644 index 00000000..dbb5984d --- /dev/null +++ b/openvidu-browser/src/OpenVidu/EventDispatcher.ts @@ -0,0 +1,101 @@ +/* + * (C) Copyright 2017-2020 OpenVidu (https://openvidu.io) + * + * 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. + * + */ + +import { Event as Event } from '../OpenViduInternal/Events/Event'; +import EventEmitter = require('wolfy87-eventemitter'); + +export abstract class EventDispatcher { + + /** + * @hidden + */ + userHandlerArrowHandler: WeakMap<(event: Event) => void, (event: Event) => void> = new WeakMap(); + /** + * @hidden + */ + ee = new EventEmitter(); + + /** + * Adds function `handler` to handle event `type` + * + * @returns The EventDispatcher object + */ + abstract on(type: string, handler: (event: Event) => void): EventDispatcher; + + /** + * Adds function `handler` to handle event `type` just once. The handler will be automatically removed after first execution + * + * @returns The object that dispatched the event + */ + abstract once(type: string, handler: (event: Event) => void): EventDispatcher; + + /** + * Removes a `handler` from event `type`. If no handler is provided, all handlers will be removed from the event + * + * @returns The object that dispatched the event + */ + off(type: string, handler?: (event: Event) => void): EventDispatcher { + if (!handler) { + this.ee.removeAllListeners(type); + } else { + // Must remove internal arrow function handler paired with user handler + const arrowHandler = this.userHandlerArrowHandler.get(handler); + if (!!arrowHandler) { + this.ee.off(type, arrowHandler); + } + this.userHandlerArrowHandler.delete(handler); + } + return this; + } + + /** + * @hidden + */ + onAux(type: string, message: string, handler: (event: Event) => void): EventDispatcher { + const arrowHandler = event => { + if (event) { + console.info(message, event); + } else { + console.info(message); + } + handler(event); + }; + this.userHandlerArrowHandler.set(handler, arrowHandler); + this.ee.on(type, arrowHandler); + return this; + } + + /** + * @hidden + */ + onceAux(type: string, message: string, handler: (event: Event) => void): EventDispatcher { + const arrowHandler = event => { + if (event) { + console.info(message, event); + } else { + console.info(message); + } + handler(event); + // Remove handler from map after first and only execution + this.userHandlerArrowHandler.delete(handler); + }; + this.userHandlerArrowHandler.set(handler, arrowHandler); + this.ee.once(type, arrowHandler); + return this; + } + +} \ No newline at end of file diff --git a/openvidu-browser/src/OpenVidu/Publisher.ts b/openvidu-browser/src/OpenVidu/Publisher.ts index 5d0e251b..bc6413c6 100644 --- a/openvidu-browser/src/OpenVidu/Publisher.ts +++ b/openvidu-browser/src/OpenVidu/Publisher.ts @@ -19,7 +19,7 @@ import { OpenVidu } from './OpenVidu'; import { Session } from './Session'; import { Stream } from './Stream'; import { StreamManager } from './StreamManager'; -import { EventDispatcher } from '../OpenViduInternal/Interfaces/Public/EventDispatcher'; +import { EventDispatcher } from './EventDispatcher'; import { PublisherProperties } from '../OpenViduInternal/Interfaces/Public/PublisherProperties'; import { Event } from '../OpenViduInternal/Events/Event'; import { StreamEvent } from '../OpenViduInternal/Events/StreamEvent'; diff --git a/openvidu-browser/src/OpenVidu/Session.ts b/openvidu-browser/src/OpenVidu/Session.ts index 74ee2bb6..5acf3363 100644 --- a/openvidu-browser/src/OpenVidu/Session.ts +++ b/openvidu-browser/src/OpenVidu/Session.ts @@ -23,7 +23,7 @@ import { Stream } from './Stream'; import { StreamManager } from './StreamManager'; import { Subscriber } from './Subscriber'; import { Capabilities } from '../OpenViduInternal/Interfaces/Public/Capabilities'; -import { EventDispatcher } from '../OpenViduInternal/Interfaces/Public/EventDispatcher'; +import { EventDispatcher } from './EventDispatcher'; import { SignalOptions } from '../OpenViduInternal/Interfaces/Public/SignalOptions'; import { SubscriberProperties } from '../OpenViduInternal/Interfaces/Public/SubscriberProperties'; import { ConnectionOptions } from '../OpenViduInternal/Interfaces/Private/ConnectionOptions'; @@ -40,7 +40,6 @@ import { StreamPropertyChangedEvent } from '../OpenViduInternal/Events/StreamPro import { OpenViduError, OpenViduErrorName } from '../OpenViduInternal/Enums/OpenViduError'; import { VideoInsertMode } from '../OpenViduInternal/Enums/VideoInsertMode'; -import EventEmitter = require('wolfy87-eventemitter'); import platform = require('platform'); /** @@ -65,7 +64,7 @@ import platform = require('platform'); * - reconnected * */ -export class Session implements EventDispatcher { +export class Session extends EventDispatcher { /** * Local connection to the Session. This object is defined only after [[Session.connect]] has been successfully executed, and can be retrieved subscribing to `connectionCreated` event @@ -131,12 +130,11 @@ export class Session implements EventDispatcher { */ stopSpeakingEventsEnabledOnce = false; - private ee = new EventEmitter(); - /** * @hidden */ constructor(openvidu: OpenVidu) { + super(); this.openvidu = openvidu; } @@ -589,14 +587,7 @@ export class Session implements EventDispatcher { */ on(type: string, handler: (event: SessionDisconnectedEvent | SignalEvent | StreamEvent | ConnectionEvent | PublisherSpeakingEvent | RecordingEvent) => void): EventDispatcher { - this.ee.on(type, event => { - if (event) { - console.info("Event '" + type + "' triggered by 'Session'", event); - } else { - console.info("Event '" + type + "' triggered by 'Session'"); - } - handler(event); - }); + super.onAux(type, "Event '" + type + "' triggered by 'Session'", handler); if (type === 'publisherStartSpeaking') { this.startSpeakingEventsEnabled = true; @@ -628,14 +619,7 @@ export class Session implements EventDispatcher { */ once(type: string, handler: (event: SessionDisconnectedEvent | SignalEvent | StreamEvent | ConnectionEvent | PublisherSpeakingEvent | RecordingEvent) => void): Session { - this.ee.once(type, event => { - if (event) { - console.info("Event '" + type + "' triggered once by 'Session'", event); - } else { - console.info("Event '" + type + "' triggered once by 'Session'"); - } - handler(event); - }); + super.onceAux(type, "Event '" + type + "' triggered once by 'Session'", handler); if (type === 'publisherStartSpeaking') { this.startSpeakingEventsEnabledOnce = true; @@ -666,11 +650,8 @@ export class Session implements EventDispatcher { * See [[EventDispatcher.off]] */ off(type: string, handler?: (event: SessionDisconnectedEvent | SignalEvent | StreamEvent | ConnectionEvent | PublisherSpeakingEvent | RecordingEvent) => void): Session { - if (!handler) { - this.ee.removeAllListeners(type); - } else { - this.ee.off(type, handler); - } + + super.off(type, handler); if (type === 'publisherStartSpeaking') { let remainingStartSpeakingListeners = this.ee.getListeners(type).length; diff --git a/openvidu-browser/src/OpenVidu/Stream.ts b/openvidu-browser/src/OpenVidu/Stream.ts index 47f5eb74..3a7dfd06 100644 --- a/openvidu-browser/src/OpenVidu/Stream.ts +++ b/openvidu-browser/src/OpenVidu/Stream.ts @@ -21,7 +21,7 @@ import { Filter } from './Filter'; import { Session } from './Session'; import { StreamManager } from './StreamManager'; import { Subscriber } from './Subscriber'; -import { EventDispatcher } from '../OpenViduInternal/Interfaces/Public/EventDispatcher'; +import { EventDispatcher } from './EventDispatcher'; import { InboundStreamOptions } from '../OpenViduInternal/Interfaces/Private/InboundStreamOptions'; import { OutboundStreamOptions } from '../OpenViduInternal/Interfaces/Private/OutboundStreamOptions'; import { WebRtcPeer, WebRtcPeerSendonly, WebRtcPeerRecvonly, WebRtcPeerSendrecv } from '../OpenViduInternal/WebRtcPeer/WebRtcPeer'; @@ -35,7 +35,6 @@ import { OpenViduError, OpenViduErrorName } from '../OpenViduInternal/Enums/Open * @hidden */ import hark = require('hark'); -import EventEmitter = require('wolfy87-eventemitter'); import platform = require('platform'); @@ -44,7 +43,7 @@ import platform = require('platform'); * Each [[Publisher]] and [[Subscriber]] has an attribute of type Stream, as they give access * to one of them (sending and receiving it, respectively) */ -export class Stream implements EventDispatcher { +export class Stream extends EventDispatcher { /** * The Connection object that is publishing the stream @@ -128,11 +127,6 @@ export class Stream implements EventDispatcher { */ filter: Filter; - /** - * @hidden - */ - ee = new EventEmitter(); - private webRtcPeer: WebRtcPeer; private mediaStream: MediaStream; private webRtcStats: WebRtcStats; @@ -206,6 +200,8 @@ export class Stream implements EventDispatcher { */ constructor(session: Session, options: InboundStreamOptions | OutboundStreamOptions | {}) { + super(); + this.session = session; if (options.hasOwnProperty('id')) { @@ -265,14 +261,7 @@ export class Stream implements EventDispatcher { * See [[EventDispatcher.on]] */ on(type: string, handler: (event: Event) => void): EventDispatcher { - this.ee.on(type, event => { - if (event) { - console.info("Event '" + type + "' triggered by stream '" + this.streamId + "'", event); - } else { - console.info("Event '" + type + "' triggered by stream '" + this.streamId + "'"); - } - handler(event); - }); + super.onAux(type, "Event '" + type + "' triggered by stream '" + this.streamId + "'", handler); return this; } @@ -281,14 +270,7 @@ export class Stream implements EventDispatcher { * See [[EventDispatcher.once]] */ once(type: string, handler: (event: Event) => void): EventDispatcher { - this.ee.once(type, event => { - if (event) { - console.info("Event '" + type + "' triggered once by stream '" + this.streamId + "'", event); - } else { - console.info("Event '" + type + "' triggered once by stream '" + this.streamId + "'"); - } - handler(event); - }); + super.onceAux(type, "Event '" + type + "' triggered once by stream '" + this.streamId + "'", handler); return this; } @@ -297,11 +279,7 @@ export class Stream implements EventDispatcher { * See [[EventDispatcher.off]] */ off(type: string, handler?: (event: Event) => void): EventDispatcher { - if (!handler) { - this.ee.removeAllListeners(type); - } else { - this.ee.off(type, handler); - } + super.off(type, handler); return this; } diff --git a/openvidu-browser/src/OpenVidu/StreamManager.ts b/openvidu-browser/src/OpenVidu/StreamManager.ts index 5467ce60..1bf157ab 100644 --- a/openvidu-browser/src/OpenVidu/StreamManager.ts +++ b/openvidu-browser/src/OpenVidu/StreamManager.ts @@ -16,14 +16,13 @@ */ import { Stream } from './Stream'; -import { EventDispatcher } from '../OpenViduInternal/Interfaces/Public/EventDispatcher'; +import { EventDispatcher } from './EventDispatcher'; import { StreamManagerVideo } from '../OpenViduInternal/Interfaces/Public/StreamManagerVideo'; import { Event } from '../OpenViduInternal/Events/Event'; import { StreamManagerEvent } from '../OpenViduInternal/Events/StreamManagerEvent'; import { VideoElementEvent } from '../OpenViduInternal/Events/VideoElementEvent'; import { VideoInsertMode } from '../OpenViduInternal/Enums/VideoInsertMode'; -import EventEmitter = require('wolfy87-eventemitter'); import platform = require('platform'); /** @@ -42,7 +41,7 @@ import platform = require('platform'); * - streamAudioVolumeChange ([[StreamManagerEvent]]) * */ -export class StreamManager implements EventDispatcher { +export class StreamManager extends EventDispatcher { /** * The Stream represented in the DOM by the Publisher/Subscriber @@ -87,20 +86,17 @@ export class StreamManager implements EventDispatcher { * @hidden */ element: HTMLElement; - /** - * @hidden - */ - ee = new EventEmitter(); /** * @hidden */ protected canPlayListener: EventListener; - /** * @hidden */ constructor(stream: Stream, targetElement?: HTMLElement | string) { + super(); + this.stream = stream; this.stream.streamManager = this; this.remote = !this.stream.isLocal(); @@ -149,14 +145,9 @@ export class StreamManager implements EventDispatcher { * See [[EventDispatcher.on]] */ on(type: string, handler: (event: Event) => void): EventDispatcher { - this.ee.on(type, event => { - if (event) { - console.info("Event '" + type + "' triggered by '" + (this.remote ? 'Subscriber' : 'Publisher') + "'", event); - } else { - console.info("Event '" + type + "' triggered by '" + (this.remote ? 'Subscriber' : 'Publisher') + "'"); - } - handler(event); - }); + + super.onAux(type, "Event '" + type + "' triggered by '" + (this.remote ? 'Subscriber' : 'Publisher') + "'", handler) + if (type === 'videoElementCreated') { if (!!this.stream && this.lazyLaunchVideoElementCreatedEvent) { this.ee.emitEvent('videoElementCreated', [new VideoElementEvent(this.videos[0].video, this, 'videoElementCreated')]); @@ -183,14 +174,9 @@ export class StreamManager implements EventDispatcher { * See [[EventDispatcher.once]] */ once(type: string, handler: (event: Event) => void): StreamManager { - this.ee.once(type, event => { - if (event) { - console.info("Event '" + type + "' triggered once by '" + (this.remote ? 'Subscriber' : 'Publisher') + "'", event); - } else { - console.info("Event '" + type + "' triggered once by '" + (this.remote ? 'Subscriber' : 'Publisher') + "'"); - } - handler(event); - }); + + super.onceAux(type, "Event '" + type + "' triggered once by '" + (this.remote ? 'Subscriber' : 'Publisher') + "'", handler); + if (type === 'videoElementCreated') { if (!!this.stream && this.lazyLaunchVideoElementCreatedEvent) { this.ee.emitEvent('videoElementCreated', [new VideoElementEvent(this.videos[0].video, this, 'videoElementCreated')]); @@ -216,11 +202,8 @@ export class StreamManager implements EventDispatcher { * See [[EventDispatcher.off]] */ off(type: string, handler?: (event: Event) => void): StreamManager { - if (!handler) { - this.ee.removeAllListeners(type); - } else { - this.ee.off(type, handler); - } + + super.off(type, handler); if (type === 'streamAudioVolumeChange') { let remainingVolumeEventListeners = this.ee.getListeners(type).length; diff --git a/openvidu-browser/src/OpenViduInternal/Interfaces/Public/EventDispatcher.ts b/openvidu-browser/src/OpenViduInternal/Interfaces/Public/EventDispatcher.ts deleted file mode 100644 index 60856c4e..00000000 --- a/openvidu-browser/src/OpenViduInternal/Interfaces/Public/EventDispatcher.ts +++ /dev/null @@ -1,43 +0,0 @@ -/* - * (C) Copyright 2017-2020 OpenVidu (https://openvidu.io) - * - * 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. - * - */ - -import { Event as Event } from '../../Events/Event'; - -export interface EventDispatcher { - - /** - * Adds function `handler` to handle event `type` - * - * @returns The EventDispatcher object - */ - on(type: string, handler: (event: Event) => void): EventDispatcher; - - /** - * Adds function `handler` to handle event `type` just once. The handler will be automatically removed after first execution - * - * @returns The object that dispatched the event - */ - once(type: string, handler: (event: Event) => void): Object; - - /** - * Removes a `handler` from event `type`. If no handler is provided, all handlers will be removed from the event - * - * @returns The object that dispatched the event - */ - off(type: string, handler?: (event: Event) => void): Object; - -} \ No newline at end of file diff --git a/openvidu-browser/src/index.ts b/openvidu-browser/src/index.ts index 498f6729..828697bf 100644 --- a/openvidu-browser/src/index.ts +++ b/openvidu-browser/src/index.ts @@ -26,7 +26,7 @@ export { FilterEvent } from './OpenViduInternal/Events/FilterEvent'; export { Capabilities } from './OpenViduInternal/Interfaces/Public/Capabilities'; export { Device } from './OpenViduInternal/Interfaces/Public/Device'; -export { EventDispatcher } from './OpenViduInternal/Interfaces/Public/EventDispatcher'; +export { EventDispatcher } from './OpenVidu/EventDispatcher'; export { OpenViduAdvancedConfiguration } from './OpenViduInternal/Interfaces/Public/OpenViduAdvancedConfiguration'; export { PublisherProperties } from './OpenViduInternal/Interfaces/Public/PublisherProperties'; export { SignalOptions } from './OpenViduInternal/Interfaces/Public/SignalOptions';