listenToOnce creates unexpected memory leak in _listeningTo object

[outdooricon]

When using listenToOnce, I’d expect that post-trigger, I’d be free of associations between my listener and my object, much like when calling once. However, even though the event listener cleans up, the object reference is still maintained through the _listeningTo object. This has potential to create unexpected memory leaks.

An Example

A parent view creates a child view that acts as a modal. I have the parent view listenToOnce for a close event on the children views. This is the only event the parent is listening for on the child. That close event fires once and the parent view callback is satisfied. The child view has .remove() performed and everything is thought to be cleaned up. The child view is not able to be gc’d though because the parent still has a reference in _listeningTo. In a case of a long living parent view, this can become a incremental leak as I perform more view creations.

As a normal backbone user, I have no idea that a reference is maintained to the object being listened to. This would be my expectation when there are no more events being watched. I don't know that I need to perform stopListening manually.

Proposal

Differentiate listenToOnce from listenTo. Let listenToOnce perform a stopListening(childView, event) when the callback is executed… thereby removing the association if it was the only event being watched.

My very inelegant solution which strikes at the crux of the problem:

  var listenMethods = {listenTo: 'on', listenToOnce: 'once'};

  // Inversion-of-control versions of `on` and `once`. Tell *this* object to
  // listen to an event in another object ... keeping track of what it's
  // listening to.
  _.each(listenMethods, function(implementation, method) {
    Events[method] = function(obj, name, callback) {
      var listeningTo = this._listeningTo || (this._listeningTo = {});
      var id = obj._listenId || (obj._listenId = _.uniqueId('l'));
      listeningTo[id] = obj;
      if (!callback && typeof name === 'object') callback = this;

      // solution begins here
      if (implementation === 'once') {
        cachedCallback = callback;
        callback = function () {
          this.stopListening(obj, name);
          return cachedCallback.apply(this, arguments);
        }
      }

      obj[implementation](name, callback, this);
      return this;
    };
  });

Thoughts?

[malte-wessel]

+1

[outdooricon]

@jashkenas and @akre54, Is there any chance of discussion about this?

[jashkenas]

Sure -- but I'd need to take a look at it first ;)

[malte-wessel]

Here's a requirebin that reproduces the bug http://requirebin.com/?gist=6e01165ef5e153469326

[jamesplease]

I whipped up a failing unit test that demonstrates this issue over in #3341.

[akre54]

This solution looks right. @malte-wessel @jmeas can you combine the failing test and the fix into a single PR?

[jamesplease]

Yup, I'll update my PR.

[akre54]

Closed in favor of #3341