Fix another leaks on addon disabling: cuddlefish leaks globals in <br…

…owser> message manager context, leaks <browser> node. (cherry picked from commit 0acfbc1)
erikvold · Feb 14, 2012 · 42ee26e · 42ee26e
1 parent 2d7d94a
commit 42ee26e
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 10 deletions.
diff --git a/packages/api-utils/lib/channel.js b/packages/api-utils/lib/channel.js
@@ -3,6 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 const { jetpackID } = require('@packaging');
+const { when } = require('./unload');
 
 // TODO: Create a bug report and remove this workaround once it's fixed.
 // Only function needs defined in the context of the message manager window
@@ -25,14 +26,14 @@ exports.channel = function channel(scope, messageManager, address, raw) {
         }
       });
       messageManager.addMessageListener(address, listener);
-      return {
-        destroy: function () {
-          if (listener) {
-            messageManager.removeMessageListener(address, listener);
-            listener = null;
-          }
+
+      // Bug 724433: do not leak `listener` on addon disabling
+      when(function () {
+        if (listener) {
+          messageManager.removeMessageListener(address, listener);
+          listener = null;
         }
-      };
+      });
     },
     output: function output(data) {
       messageManager.sendAsyncMessage(address, data);

diff --git a/packages/api-utils/lib/cuddlefish.js b/packages/api-utils/lib/cuddlefish.js
@@ -288,7 +288,7 @@ const Loader = {
     process.spawn(id, path)(function(addon) {
       // Listen to `require!` channel's input messages from the add-on process
       // and load modules being required.
-      loader.data.channel = addon.channel('require!').input(
+      addon.channel('require!').input(
         function({ requirer: { path }, id }) {
           try {
             Loader.require.call(loader, path, id).initialize(addon.channel(id));
@@ -300,7 +300,6 @@ const Loader = {
   },
   unload: function unload(reason, callback) {
     this.require('api-utils/unload').send(reason, callback);
-    this.data.channel.destroy();
   }
 };
 exports.Loader = Loader;

diff --git a/packages/api-utils/lib/process.js b/packages/api-utils/lib/process.js
@@ -40,7 +40,11 @@ function process(target, id, path, scope) {
     // Please note that it's important to unload remote loader
     // synchronously (using synchronous frame script), to make sure that we
     // don't stop during unload.
-    loadScript(target, 'data:,loader.unload("' + reason + '")', true);
+    // Bug 724433: Take care to nullify all globals set by `cuddlefish.js`
+    // otherwise, we will leak any still defined global.
+    // `dump` is set in Loader.new method, `dump = globals.dump;`
+    loadScript(target, 'data:,loader.unload("' + reason + '");' +
+                       'loader = null; Loader = null; dump = null;', true);
   });
 
   return { channel: channel.bind(null, scope, target) }

diff --git a/packages/api-utils/lib/window-utils.js b/packages/api-utils/lib/window-utils.js
@@ -7,6 +7,7 @@
 const { Cc, Ci } = require("chrome");
 const { EventEmitter } = require('./events'),
       { Trait } = require('./traits');
+const { when } = require('./unload');
 const errors = require("./errors");
 
 const gWindowWatcher = Cc["@mozilla.org/embedcomp/window-watcher;1"].
@@ -230,6 +231,11 @@ exports.createRemoteBrowser = function createRemoteBrowser(remote) {
       browser.setAttribute("flex", "1");
       document.documentElement.appendChild(browser);
 
+      // Bug 724433: do not leak this <browser> DOM node
+      when(function () {
+        document.documentElement.removeChild(browser);
+      });
+
       // Return browser
       deliver(browser);
     });