Browse Source

Fix memory leak with extension messaging Port. Fix #383.

FelisCatus 10 years ago
parent
commit
974ef69912

+ 5 - 0
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

+ 75 - 0
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

+ 3 - 1
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)
 

+ 4 - 2
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

+ 3 - 2
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