From 974ef69912f69548b3ae3ff0269897e727fd2694 Mon Sep 17 00:00:00 2001 From: FelisCatus Date: Sat, 4 Apr 2015 16:59:50 +0800 Subject: [PATCH] Fix memory leak with extension messaging Port. Fix #383. --- .../omega_target_web.coffee | 5 ++ .../src/chrome_port.coffee | 75 +++++++++++++++++++ .../src/external_api.coffee | 4 +- .../src/options.coffee | 6 +- .../src/switchysharp.coffee | 5 +- 5 files changed, 90 insertions(+), 5 deletions(-) create mode 100644 omega-target-chromium-extension/src/chrome_port.coffee diff --git a/omega-target-chromium-extension/omega_target_web.coffee b/omega-target-chromium-extension/omega_target_web.coffee index e423007..27a7334 100644 --- a/omega-target-chromium-extension/omega_target_web.coffee +++ b/omega-target-chromium-extension/omega_target_web.coffee @@ -31,6 +31,11 @@ angular.module('omegaTarget', []).factory 'omegaTarget', ($q) -> return d.promise connectBackground = (name, message, callback) -> port = chrome.runtime.connect({name: name}) + onDisconnect = -> + port.onDisconnect.removeListener(onDisconnect) + port.onMessage.removeListener(callback) + port.onDisconnect.addListener(onDisconnect) + port.postMessage(message) port.onMessage.addListener(callback) return diff --git a/omega-target-chromium-extension/src/chrome_port.coffee b/omega-target-chromium-extension/src/chrome_port.coffee new file mode 100644 index 0000000..0cddb01 --- /dev/null +++ b/omega-target-chromium-extension/src/chrome_port.coffee @@ -0,0 +1,75 @@ +# A wrapper around type Port in Chromium Extension API. +# https://developer.chrome.com/extensions/runtime#type-Port +# +# Please wrap any Port object in this class BEFORE adding listeners. Adding +# listeners to events of raw Port objects should be avoided to minimize the risk +# of memory leaks. See the comments of the TrackedEvent class for more details. +module.exports = class ChromePort + constructor: (@port) -> + @name = @port.name + @sender = @port.sender + + @disconnect = @port.disconnect.bind(@port) + @postMessage = @port.postMessage.bind(@port) + + @onMessage = new TrackedEvent(@port.onMessage) + @onDisconnect = new TrackedEvent(@port.onDisconnect) + @onDisconnect.addListener @dispose.bind(this) + + dispose: -> + @onMessage.dispose() + @onDisconnect.dispose() + +# A wrapper around chrome.Event. +# https://developer.chrome.com/extensions/events#type-Event +# +# ALL event listeners MUST be manually removed before disposing any Event or +# object containing Event, such as Port. Otherwise, a memory leak will happen. +# https://code.google.com/p/chromium/issues/detail?id=320723 +# +# TrackedEvent helps to solve this problem by keeping track of all listeners +# installed and removes them when the #dispose method is called. +# Don't forget to call #dispose when this TrackedEvent is not needed any more. +class TrackedEvent + constructor: (@event) -> + @callbacks = [] + mes = ['hasListener', 'hasListeners', 'addRules', 'getRules', 'removeRules'] + for method in mes + this[method] = @event[method].bind(@event) + + addListener: (callback) -> + @event.addListener(callback) + @callbacks.push(callback) + return this + + removeListener: (callback) -> + @event.removeListener(callback) + i = @callbacks.indexOf(callback) + @callbacks.splice(i, 1) if i >= 0 + return this + + ###* + # Removes all listeners added via this TrackedEvent instance. + # Note: Won't remove listeners added via other TrackedEvent or raw Event. + ### + removeAllListeners: -> + for callback in @callbacks + @event.removeListener(callback) + @callbacks = [] + return this + + ###* + # Removes all listeners added via this TrackedEvent instance and prevent any + # further listeners from being added. It is considered safe to nullify any + # references to this instance and the underlying Event without causing leaks. + # This should be the last method called in the lifetime of TrackedEvent. + # + # Throws if the underlying raw Event object still has listeners. This can + # happen when listeners have been added via other TrackedEvents or raw Event. + ### + dispose: -> + @removeAllListeners() + if @event.hasListeners() + throw new Error("Underlying Event still has listeners!") + @event = null + @callbacks = null diff --git a/omega-target-chromium-extension/src/external_api.coffee b/omega-target-chromium-extension/src/external_api.coffee index 4cf7c74..3c7a92e 100644 --- a/omega-target-chromium-extension/src/external_api.coffee +++ b/omega-target-chromium-extension/src/external_api.coffee @@ -1,6 +1,7 @@ OmegaTarget = require('omega-target') OmegaPac = OmegaTarget.OmegaPac Promise = OmegaTarget.Promise +ChromePort = require('./chrome_port') module.exports = class ExternalApi constructor: (options) -> @@ -9,7 +10,8 @@ module.exports = class ExternalApi 'padekgcemlokbadohgkifijomclgjgif': 32 disabled: false listen: -> - chrome.runtime.onConnectExternal.addListener (port) => + chrome.runtime.onConnectExternal.addListener (rawPort) => + port = new ChromePort(rawPort) port.onMessage.addListener (msg) => @onMessage(msg, port) port.onDisconnect.addListener @reenable.bind(this) diff --git a/omega-target-chromium-extension/src/options.coffee b/omega-target-chromium-extension/src/options.coffee index 528ca54..bc69e53 100644 --- a/omega-target-chromium-extension/src/options.coffee +++ b/omega-target-chromium-extension/src/options.coffee @@ -8,6 +8,7 @@ proxySettings = chromeApiPromisifyAll(chrome.proxy.settings) parseExternalProfile = require('./parse_external_profile') ProxyAuth = require('./proxy_auth') WebRequestMonitor = require('./web_request_monitor') +ChromePort = require('./chrome_port') class ChromeOptions extends OmegaTarget.Options _inspect: null @@ -216,10 +217,11 @@ class ChromeOptions extends OmegaTarget.Options summary: info.summary }) - chrome.runtime.onConnect.addListener (port) => - return unless port.name == 'tabRequestInfo' + chrome.runtime.onConnect.addListener (rawPort) => + return unless rawPort.name == 'tabRequestInfo' return unless @_monitorWebRequests tabId = null + port = new ChromePort(rawPort) port.onMessage.addListener (msg) => tabId = msg.tabId @_tabRequestInfoPorts[tabId] = port diff --git a/omega-target-chromium-extension/src/switchysharp.coffee b/omega-target-chromium-extension/src/switchysharp.coffee index b5c5c9e..c2e632f 100644 --- a/omega-target-chromium-extension/src/switchysharp.coffee +++ b/omega-target-chromium-extension/src/switchysharp.coffee @@ -1,6 +1,7 @@ OmegaTarget = require('omega-target') OmegaPac = OmegaTarget.OmegaPac Promise = OmegaTarget.Promise +ChromePort = require('./chrome_port') module.exports = class SwitchySharp @extId: 'dpplabbmogkhghncfbfdeeokoefdjegm' @@ -45,9 +46,9 @@ module.exports = class SwitchySharp _connect: -> if not @port - @port = chrome.runtime.connect(SwitchySharp.extId) + @port = new ChromePort(chrome.runtime.connect(SwitchySharp.extId)) @port.onDisconnect.addListener(@_onDisconnect.bind(this)) - @port.onMessage.addListener(@_onMessage.bind(this)) + @port?.onMessage.addListener(@_onMessage.bind(this)) try @port.postMessage({action: 'disable'}) catch