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

Event handlers causing memory leak in inpage.js #6176

Closed
gkaemmer opened this issue Feb 17, 2019 · 0 comments
Closed

Event handlers causing memory leak in inpage.js #6176

gkaemmer opened this issue Feb 17, 2019 · 0 comments
Assignees

Comments

@gkaemmer
Copy link
Contributor

Hello! I just found the cause of an issue that's been bugging me for a while.

Describe the bug
There's a bug in inpage.js at line 35. Repeated calls to isApproved or isUnlocked add event listeners on window that are never removed. To properly remove event listeners, removeEventListener must be called with the exact reference to the original handler, which is not the case here.

To Reproduce
With metamask installed, open chrome devtools in a new window. Then:

getEventListeners(window).message.length
// => 3
await ethereum._metamask.isApproved()
// => false (or true, doesn't matter)
getEventListeners(window).message.length
// => 4
await ethereum._metamask.isApproved()
await ethereum._metamask.isApproved()
getEventListeners(window).message.length
// => 6

Expected behavior
Events should be cleaned up.

Screenshots
N/A

Browser details (please complete the following information):
I assume anywhere that inpage.js runs is affected by this, but I'm using the latest version of chrome and metamask 6.0.1.

Additional context
This causes memory and CPU to go off the charts in windows that have called isUnlocked or isApproved a bunch of times, which is common when polling for wallet status. Also I think there are a number of similar leaks in inpage.js--the onMessage function never successfully removes listeners.

It should be pretty trivial to fix this--let me know if you need help with it.

gkaemmer added a commit to gkaemmer/metamask-extension that referenced this issue Feb 17, 2019
gkaemmer added a commit to gkaemmer/metamask-extension that referenced this issue Feb 17, 2019
bitpshr pushed a commit that referenced this issue Feb 18, 2019
@ghost ghost assigned danfinlay Feb 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants