-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Comments
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. |
@sorvell thanks for triaging this! I will keep an eye on that issue. |
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. |
No news about this issue? In a SPA this issue is a disaster! |
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. |
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
ingestures.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
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
The text was updated successfully, but these errors were encountered: