Fix Blaze memory leak

This leak occured whenever a DOM element with attributes got removed
from the DOM without destroying its containing view.  For example, an
element in an `{{#if}}` whose condition is toggled back and forth; a
single Blaze.View is used for the `{{#if}}` as its condition changes.

The retention chain was as follows:

The element was saved in the `elem` variable of `materializeTag` in
materializer.js.

Among other places, `elem` was saved in the `attrUpdater` in that
function, which is used by the `updateAttributes` function.

This function is passed to `view.autorun`, which registered this
`onViewDestroyed` handler:

   self.onViewDestroyed(function () { locals.c.stop(); });

That callback retains a references to the computation, and thus to the
DOM element.

Before this commit, that onViewDestroyed callback is not removed from
the Blaze.View when the computation is stopped (eg, because the DOM
element is removed from the DOM, triggering
`updaterComputation.stop()`), so in this case the DOM element is leaked.

(This still has a tiny leak in that you end up with lots of nulls
inside `_callbacks.destroyed`; using a better data structure for
callbacks is a game for another day.)

Fixes #4289.
This commit is contained in:
David Glasser
2015-04-29 00:19:30 -07:00
parent 91ce1561c3
commit 583d77e943

View File

@@ -123,6 +123,19 @@ Blaze.View.prototype.onViewDestroyed = function (cb) {
this._callbacks.destroyed = this._callbacks.destroyed || [];
this._callbacks.destroyed.push(cb);
};
Blaze.View.prototype.removeViewDestroyedListener = function (cb) {
var destroyed = this._callbacks.destroyed;
if (! destroyed)
return;
var index = _.lastIndexOf(destroyed, cb);
if (index !== -1) {
// XXX You'd think the right thing to do would be splice, but _fireCallbacks
// gets sad if you remove callbacks while iterating over the list. Should
// change this to use callback-hook or EventEmitter or something else that
// properly supports removal.
destroyed[index] = null;
}
};
/// View#autorun(func)
///
@@ -203,7 +216,11 @@ Blaze.View.prototype.autorun = function (f, _inViewScope, displayName) {
(self.name || 'anonymous') + ':' + (displayName || 'anonymous');
locals.c = Tracker.autorun(locals.f);
self.onViewDestroyed(function () { locals.c.stop(); });
var stopComputation = function () { locals.c.stop(); };
self.onViewDestroyed(stopComputation);
locals.c.onStop(function () {
self.removeViewDestroyedListener(stopComputation);
});
return locals.c;
};
@@ -267,7 +284,7 @@ Blaze._fireCallbacks = function (view, which) {
Tracker.nonreactive(function fireCallbacks() {
var cbs = view._callbacks[which];
for (var i = 0, N = (cbs && cbs.length); i < N; i++)
cbs[i].call(view);
cbs[i] && cbs[i].call(view);
});
});
};