tooltip: memory leak on destroy

[pbininda]

Bug, feature request, or proposal:

commit #11087 introduced a memory leak in md-tooltip

What is the expected behavior?

no memory leak

What is the current behavior?

memory leak in md-tooltip

CodePen and steps to reproduce the issue:

CodePen Demo which shows your issue:

https://codepen.io/pbininda/pen/vdrZZX

Detailed Reproduction Steps:

Step 1: No memory leaked

• load the CodePen in Chrome and open chrome dev tools
• Hit the "TOGGLE BUTTON" 10 times (making the ng-if shown true 5 times)
• In the dev tools take a memory snapshot and search for "Canary"
• No living instances of "Canary" are found.

Step 2: Trigger memory leak

• load the CodePen in Chrome and open chrome dev tools
• Hit the "TOGGLE BUTTON" 10 times (making the ng-if shown true 5 times)
  • Each time when the "BUTTON WITH TOOLTIP" is shown, hover over the "BUTTON WITH TOOLTIP" to show the "tooltip"
• In the dev tools take a memory snapshot and search for "Canary"
• 5 live instances of "Canary" are shown.

What is the use-case or motivation for changing an existing behavior?

We have a big application using angular-material. This memory leak keeps a huge chain of objects alive whenever a user hovers over a button.

Which versions of AngularJS, Material, OS, and browsers are affected?

Material version 1.1.7

Is there anything else we should know? Stack Traces, Screenshots, etc.

Reverting the commit #11087 fixes the problem.

Below is a screenshot of the heap snapshot showing 5 leaked instances of "Canary"

[fsmeier]

Are you sure it was the linked commit? - I do not see anything which could lead to memory leak.
But maybe we should take a look at #10651
Edit: but also there it does not look like something which could introduce missing memory cleanup

[pbininda]

Looking at the retainers chain (see screenshot), it seems the onDomAdded callback function that is registered by the tooltip, keeps the $scope object alive through the closure.
Anyway, if I undo the changes of the linked commit, the leak goes away.

[fsmeier]

Ok, I can now reproduce the problem, but the problem seems like not the tooltip itself. If I replace the onDomAdded-function with the following i have still the leak.

onDomAdded: function() {
            var i = 1 + 3;
          }

Update of research: ANY function passed inside the config leads to this problem.

[fsmeier]

I still think the linked commit is not the problem!
The last two hours I tracked the problem down and I found some problems which just got visible whenever the config has ANY closures.

1. The config together with scope is cached in the trackedPanels-object with the following two lines:

  var panelRef = new MdPanelRef(this._config, this._$injector);
  this._trackedPanels[config.id] = panelRef;

There is some missbehavior with the trackedPanels and config anyway and I think the trackedPanels should handle the scope somehow. For me it is linked to #10894

2. The config for the panel is stored as object-property but I do not see any reason why. At least it should free the config-values for the garbage-collector since it has the scope linked there.

delete this._config;

So in the end for now the fastest fix without loosing any at the moment working functionality is the following create method (just the changes described above changed):

MdPanelService.prototype.create = function(preset, config) {
  if (typeof preset === 'string') {
    preset = this._getPresetByName(preset);
  } else if (typeof preset === 'object' &&
      (angular.isUndefined(config) || !config)) {
    config = preset;
    preset = {};
  }

  preset = preset || {};
  config = config || {};

  // If the passed-in config contains an ID and the ID is within _trackedPanels,
  // return the tracked panel after updating its config with the passed-in
  // config.
  if (angular.isDefined(config.id) && this._trackedPanels[config.id]) {
    var trackedPanel = this._trackedPanels[config.id];
    angular.extend(trackedPanel.config, config);
    return trackedPanel;
  }

  // Combine the passed-in config, the _defaultConfigOptions, and the preset
  // configuration into the `_config`.
  this._config = angular.extend({
    // If no ID is set within the passed-in config, then create an arbitrary ID.
    id: config.id || 'panel_' + this._$mdUtil.nextUid(),
    scope: this._$rootScope.$new(true),
    attachTo: this._$rootElement
  }, this._defaultConfigOptions, config, preset);

  // Create the panelRef and add it to the `_trackedPanels` object.
  var panelRef = new MdPanelRef(this._config, this._$injector);
  // removing the following line fixes the multiple memory leak of the scope
  this._trackedPanels[config.id] = panelRef;

  // Add the panel to each of its requested groups.
  if (this._config.groupName) {
    if (angular.isString(this._config.groupName)) {
      this._config.groupName = [this._config.groupName];
    }
    angular.forEach(this._config.groupName, function(group) {
      panelRef.addToGroup(group);
    });
  }

  this._config.scope.$on('$destroy', angular.bind(panelRef, panelRef.detach));

 // adding the following line fixes the scope in the memory even if the tooltip is not there anymore
  delete this._config;

  return panelRef;
};

PS: I do not have time until the 5.3.2018 to fix the complete trackedPanels-messup which would fix this as well.

[Splaktar]

Can someone please test #11145 to see if it solves this leak for you?

[fsmeier]

I will test it on monday.

[pbininda]

I manually applied the change from #11145 to my local installation and can confirm, that it fixes the leak for me.
👍