Bug 1267693 P1 Cache the most recent event in a channel weakly to avoid leaking DOM objects. r=gabor a=sylvestre
authorBen Kelly <ben@wanderview.com>
Fri, 22 Jul 2016 10:17:50 -0700
changeset 340024 2f9a3a609582d45b1adee9150ff86d3e3711f759
parent 340023 8fd80dfc8f7df45c301780c94650e66c06ef077e
child 340025 d0deaae22a870e4cdd3167b52ed8777335ddf700
push id6249
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 13:59:36 +0000
treeherdermozilla-beta@bad9d4f5bf7e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgabor, sylvestre
bugs1267693
milestone49.0a2
Bug 1267693 P1 Cache the most recent event in a channel weakly to avoid leaking DOM objects. r=gabor a=sylvestre
addon-sdk/source/lib/sdk/event/utils.js
--- a/addon-sdk/source/lib/sdk/event/utils.js
+++ b/addon-sdk/source/lib/sdk/event/utils.js
@@ -3,16 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 "use strict";
 
 module.metadata = {
   "stability": "unstable"
 };
 
 var { emit, on, once, off, EVENT_TYPE_PATTERN } = require("./core");
+const { Cu } = require("chrome");
 
 // This module provides set of high order function for working with event
 // streams (streams in a NodeJS style that dispatch data, end and error
 // events).
 
 // Function takes a `target` object and returns set of implicit references
 // (non property references) it keeps. This basically allows defining
 // references between objects without storing the explicitly. See transform for
@@ -21,17 +22,17 @@ var refs = (function() {
   let refSets = new WeakMap();
   return function refs(target) {
     if (!refSets.has(target)) refSets.set(target, new Set());
     return refSets.get(target);
   };
 })();
 
 function transform(input, f) {
-  let output = {};
+  let output = new Output();
 
   // Since event listeners don't prevent `input` to be GC-ed we wanna presrve
   // it until `output` can be GC-ed. There for we add implicit reference which
   // is removed once `input` ends.
   refs(output).add(input);
 
   const next = data => receive(output, data);
   once(output, "start", () => start(input));
@@ -59,17 +60,17 @@ exports.filter = filter;
 // mapped via given `f` function.
 const map = (input, f) => transform(input, (data, next) => next(f(data)));
 exports.map = map;
 
 // High order function that takes `input` stream of streams and merges them
 // into single event stream. Like flatten but time based rather than order
 // based.
 function merge(inputs) {
-  let output = {};
+  let output = new Output();
   let open = 1;
   let state = [];
   output.state = state;
   refs(output).add(inputs);
 
   function end(input) {
     open = open - 1;
     refs(output).delete(input);
@@ -102,23 +103,28 @@ exports.merge = merge;
 const expand = (inputs, f) => merge(map(inputs, f));
 exports.expand = expand;
 
 const pipe = (from, to) => on(from, "*", emit.bind(emit, to));
 exports.pipe = pipe;
 
 
 // Shim signal APIs so other modules can be used as is.
-
 const receive = (input, message) => {
   if (input[receive])
     input[receive](input, message);
   else
     emit(input, "data", message);
 
+  // Ideally our input will extend Input and already provide a weak value
+  // getter.  If not, opportunistically shim the weak value getter on
+  // other types passed as the input.
+  if (!("value" in input)) {
+    Object.defineProperty(input, "value", WeakValueGetterSetter);
+  }
   input.value = message;
 };
 receive.toString = () => "@@receive";
 exports.receive = receive;
 exports.send = receive;
 
 const end = input => {
   if (input[end])
@@ -146,17 +152,17 @@ const start = input => {
 };
 start.toString = () => "@@start";
 exports.start = start;
 
 const lift = (step, ...inputs) => {
   let args = null;
   let opened = inputs.length;
   let started = false;
-  const output = {};
+  const output = new Output();
   const init = () => {
     args = [...inputs.map(input => input.value)];
     output.value = step(...args);
   };
 
   inputs.forEach((input, index) => {
     on(input, "data", data => {
       args[index] = data;
@@ -177,17 +183,18 @@ const lift = (step, ...inputs) => {
   init();
 
   return output;
 };
 exports.lift = lift;
 
 const merges = inputs => {
   let opened = inputs.length;
-  let output = { value: inputs[0].value };
+  let output = new Output();
+  output.value = inputs[0].value;
   inputs.forEach((input, index) => {
     on(input, "data", data => receive(output, data));
     on(input, "end", () => {
       opened = opened - 1;
       if (opened <= 0)
         end(output);
     });
   });
@@ -220,22 +227,56 @@ Input.start = input => emit(input, "star
 Input.prototype.start = Input.start;
 
 Input.end = input => {
   emit(input, "end", input);
   stop(input);
 };
 Input.prototype[end] = Input.end;
 
+// The event channel system caches the last event seen as input.value.
+// Unfortunately, if the last event is a DOM object this is a great way
+// leak windows.  Mitigate this by storing input.value using a weak
+// reference.  This allows the system to work for normal event processing
+// while also allowing the objects to be reclaimed.  It means, however,
+// input.value cannot be accessed long after the event was dispatched.
+const WeakValueGetterSetter = {
+  get: function() {
+    return this._weakValue ? this._weakValue.get() : this._simpleValue
+  },
+  set: function(v) {
+    if (v && typeof v === "object") {
+      this._weakValue = Cu.getWeakReference(v)
+      this._simpleValue = undefined;
+      return;
+    }
+    this._simpleValue = v;
+    this._weakValue = undefined;
+  },
+}
+Object.defineProperty(Input.prototype, "value", WeakValueGetterSetter);
+
 exports.Input = Input;
 
+// Define an Output type with a weak value getter for the transformation
+// functions that produce new channels.
+function Output() { }
+Object.defineProperty(Output.prototype, "value", WeakValueGetterSetter);
+exports.Output = Output;
+
 const $source = "@@source";
 const $outputs = "@@outputs";
 exports.outputs = $outputs;
 
+// NOTE: Passing DOM objects through a Reactor can cause them to leak
+// when they get cached in this.value.  We cannot use a weak reference
+// in this case because the Reactor design expects to always have both the
+// past and present value.  If we allow past values to be collected the
+// system breaks.
+
 function Reactor(options={}) {
   const {onStep, onStart, onEnd} = options;
   if (onStep)
     this.onStep = onStep;
   if (onStart)
     this.onStart = onStart;
   if (onEnd)
     this.onEnd = onEnd;