diff options
author | John Vilk <jvilk@cs.umass.edu> | 2017-03-27 18:06:24 +0300 |
---|---|---|
committer | Stefan Giehl <stefan@piwik.org> | 2017-03-27 18:06:24 +0300 |
commit | dc37bf7960583b89071ca770e81041c2cdefa902 (patch) | |
tree | 17f851563cbd975be2ed53eaf6ab69b1efed1e74 /plugins/CoreVisualizations | |
parent | 0f887dcb0bb5583d06a20eedd4db20585eff16eb (diff) |
Fix memory leaks in data table / jqplot (#11354)
* Fixes memory leaks in dataTable and jqplot via uncleared resize handlers
Previously, these plugins registered a handler on the `resize` event on window,
but used ad-hoc heuristics to determine when to remove the handler:
* dataTable would only remove the handler the next time the `resize` event fires,
which is not guaranteed to happen regularly (especially if the user is not
regularly resizing their browser window, e.g., it is full screened)
* Fix: I define `_destroy` on `dataTable`, which appropriately cleans up the
handler (and calls the `_destroy` method of its parent class, `UIControl`)
* jqplot contained code to remove the `resize` listener when `destroyPlot()`
is called, but a) this function is not called when the plot is removed from
the DOM (the code called `destroy()` on the `plot` object directly), and
b) it incorrectly uses `this` instead of `self`, preventing it from working
in the first place.
* Fix: I fixed the `this`/`self` confusion, and changed the cleanup code
such that `$.jqplot.visiblePlots` contains `JqplotGraphDataTable` objects
rather than `jqplot` objects.
These event handlers prevented previous jqplot and datatable objects from being
garbage collected each time the dashboard is refreshed.
* Prevent leaking data tables via body mouseup handlers.
Adds code to clean up mouseup handlers on the body HTML element when data tables are destroyed.
Previously, these handlers caused data tables to remain in memory forever.
Diffstat (limited to 'plugins/CoreVisualizations')
-rw-r--r-- | plugins/CoreVisualizations/javascripts/jqplot.js | 12 |
1 files changed, 6 insertions, 6 deletions
diff --git a/plugins/CoreVisualizations/javascripts/jqplot.js b/plugins/CoreVisualizations/javascripts/jqplot.js index e840a71537..13c91a568c 100644 --- a/plugins/CoreVisualizations/javascripts/jqplot.js +++ b/plugins/CoreVisualizations/javascripts/jqplot.js @@ -178,12 +178,12 @@ // manage resources target.on('piwikDestroyPlot', function () { - if (this._resizeListener) { - $(window).off('resize', this._resizeListener); + if (self._resizeListener) { + $(window).off('resize', self._resizeListener); } self._plot.destroy(); for (var i = 0; i < $.jqplot.visiblePlots.length; i++) { - if ($.jqplot.visiblePlots[i] == self._plot) { + if ($.jqplot.visiblePlots[i] === self) { $.jqplot.visiblePlots[i] = null; } } @@ -394,14 +394,14 @@ if ($.jqplot.visiblePlots[i] == null) { continue; } - $.jqplot.visiblePlots[i].destroy(); + $.jqplot.visiblePlots[i].destroyPlot(); } $.jqplot.visiblePlots = []; }); } if (typeof plot != 'undefined') { - $.jqplot.visiblePlots.push(plot); + $.jqplot.visiblePlots.push(self); } }, @@ -1196,4 +1196,4 @@ RowEvolutionSeriesToggle.prototype.beforeReplot = function () { $.jqplot.preInitHooks.push($.jqplot.PieLegend.init); $.jqplot.postDrawHooks.push($.jqplot.PieLegend.postDraw); -})(jQuery, require);
\ No newline at end of file +})(jQuery, require); |