New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JavaScript Memory Leak: widgetContent $destroy handlers #12059

Open
jvilk opened this Issue Sep 14, 2017 · 0 comments

Comments

Projects
None yet
2 participants
@jvilk
Contributor

jvilk commented Sep 14, 2017

This is a somewhat complex memory leak made even more complex by my only vague familiarity with AngularJS. I will do my best to explain what is going on!

Steps to Reproduce

  • Visit the Dashboard of a Piwik installation.
  • Open up the devtools console, and inspect the length of the following property:
    • window.$widgetContent[0].jQuery[long number].$scope.$$listeners.$destroy
    • Note: jQuery randomly generates the long number.
    • Note 2: You have two instances of jQuery on the page, which both use different numbers in their properties. Typically, the larger of the two has the $scope property.
  • Click on the "Dashboard" link in the menu to refresh the dashboard.
  • Re-inspect the property, noting how it has doubled in length.

Cause

compileAngularComponents is to blame; specifically, this line:

https://github.com/piwik/piwik/blob/9c7d7ff1c842132bc8f3f0ff8c296eb7d3959c30/plugins/Morpheus/javascripts/piwikHelper.js#L123

The call to $compile causes AngularJS to register the $destroy handler on the element's scope. However, the scope is never destroyed -- it is re-used across dashboard visits.

Solution

Is it possible to destroy and re-create the scope when you redraw the dashboard? If not, then perhaps you can remove all of these listeners as a hackfix when the dashboard is redrawn?

Alternatively, are you supposed to re-compile these widgets when you revisit the dashboard? I honestly don't know. :-)

Impact

To measure the impact of this leak on Piwik's heap size, I hackfixed the leak by simply removing the line in Angular JS that adds the listener. Here is the result:

piwik_angular_leak

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment