Skip to content
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

Memory leak from _handleNative with ShadyDOM #5545

Open
2 of 4 tasks
Legioth opened this issue May 21, 2019 · 5 comments
Open
2 of 4 tasks

Memory leak from _handleNative with ShadyDOM #5545

Legioth opened this issue May 21, 2019 · 5 comments
Labels

Comments

@Legioth
Copy link

Legioth commented May 21, 2019

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

  • Chrome
  • Firefox
  • Edge
  • IE 11

Versions

  • Polymer: v2.7.2
  • webcomponents: v1.3.3
@sorvell
Copy link
Contributor

sorvell commented Jul 15, 2019

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 webcomponents/polyfills#175.

@web-padawan
Copy link
Contributor

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

@stale
Copy link

stale bot commented Jul 14, 2020

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.

@stale stale bot added the wontfix label Jul 14, 2020
@Eymerich01
Copy link

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

@stale stale bot removed the wontfix label Nov 24, 2020
@stale
Copy link

stale bot commented Mar 2, 2022

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.

@stale stale bot added the wontfix label Mar 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants