search

Polymer / polymer

Our original Web Component library.
https://polymer-library.polymer-project.org/
BSD 3-Clause "New" or "Revised" License
22.04k stars 2.01k forks source link

Memory leak from _handleNative with ShadyDOM #5545

Open Legioth opened 5 years ago

Legioth commented 5 years ago

Description

The Shady DOM polyfill overrides addEventListener and among other things stores some metadata as an extra property on the event handler function or object. This causes memory leaks when the same event listener is used for multiple elements, unless you're careful to explicitly remove all listeners when the element is no longer used.

It could be argued that this is a bug in Shady DOM, but a comment in the code implies that changing it to avoid leaks would have undesirable effects on performance: https://github.com/webcomponents/shadydom/blob/f7591f7e72a6536b5d7752c493df108a5f47f4bb/src/patch-events.js#L429-L433

This causes problems with _handleNative in gestures.js since the same function instance is used for all elements for which some specific listeners are registered, thus preventing all such elements from being garbage collected. Furthermore, adding _handleNative as a listener becomes slower and slower as the array of metadata instances keeps growing.

To work around this particular case, Polymer could create a per-node function that delegates to _handleNative and use that function instead of directly adding _handleNative as the event listener.

The problem can be observed with a variety of existing elements such as <paper-button> or <vaadin-grid>.

Live Demo

https://jsbin.com/domimaxidi/1/edit?html,console,output

Steps to Reproduce

  1. Open the live demo in Edge (also reproduces in IE 11, but my example won't load properly there)
  2. Click the element repeatedly to create more and more leaking instances

Expected Results

Taking regular random fluctuations into account, the displayed time information should remain constant.

Actual Results

In Edge, the time taken to perform the action starts creeping upwards with repeated clicks. On my machine, it starts out at around 30 ms and then keeps increasing so that the 10th click takes around 70 ms and the 20th is around 110 ms.

Browsers Affected

Versions

sorvell commented 5 years ago

Sorry for the delay on this. It's a very good finding and something we definitely need to fix. We're going to address in the ShadyDOM polyfill itself, see https://github.com/webcomponents/polyfills/issues/175.

web-padawan commented 5 years ago

@sorvell thanks for triaging this! I will keep an eye on that issue.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Eymerich01 commented 3 years ago

No news about this issue? In a SPA this issue is a disaster!

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.