Skip to content

Commit

Permalink
Change Element.removeData to actually remove keys from data map
Browse files Browse the repository at this point in the history
Without this change, it is very easy for an application to slowly leak memory as Raphael continues to add new keys to `eldata`.

This memory leak turns out to be a problem for [Piwik](https://piwik.org/), which uses
kartographer.js (which uses raphael) to draw an SVG image of the United States
in its analytics dashboard ([see demo](https://demo.piwik.org/index.php?module=CoreHome&action=index&idSite=3&period=day&date=yesterday)).

Every time the dashboard is redrawn, it spawns ~700 new Elements with data. [I've submitted a pull request that appropriately cleans up this data when the user navigates away from the page using `removeData`](matomo-org/matomo#11350), but Raphael leaves an empty object in the `eldata` map keyed on the element's old ID. Since kartographer/Piwik does not reuse IDs, `eldata` will grow continually on each redraw until the application is out of memory.
  • Loading branch information
John Vilk committed Feb 19, 2017
1 parent fe8e591 commit 29c3423
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion dev/raphael.core.js
Original file line number Diff line number Diff line change
Expand Up @@ -3120,7 +3120,7 @@ define(["eve"], function(eve) {
\*/
elproto.removeData = function (key) {
if (key == null) {
eldata[this.id] = {};
delete eldata[this.id];
} else {
eldata[this.id] && delete eldata[this.id][key];
}
Expand Down

0 comments on commit 29c3423

Please sign in to comment.