Bug Bounty: High memory usage, over 2 GB taken by MetaMask

jchittoda commented 6 years ago

Bounty: MetaMask has received multiple reports of the extension running high CPU usage. This bounty is to identify the root cause of the issue and write a patch.

MetaMask showed high memory and CPU usage. My laptop become unresponsive. When I killed MetaMask from Google Chrome's Task Manger window, then my laptop become responsive. I was on Kovan network, could this be an issue, because blocktime for Kovan (PoA) is 5 seconds.

Actual Behavior

I kept my laptop running overnight, and when wakeup in morning, my laptop fan was running in full throttle, when I tried to find, which process is taking that much resources. I found Chrome and when opened TaskManager in Chrome, I found MetaMask is the culprit.

Expected Behavior

It should not take that much of memory. It should clean the memory regularly.

Browser Used

Google Chrome 65.0.3325.181 (Official Build) (64-bit)

Operating System Used

MacOS High Sierra 10.13.4

Here is the screen shot of the Chrome's Task Manager.

danfinlay commented 6 years ago

This may be related to a memory leak just discovered in provider-engine that was recently shipped. We are currently working on that, and hopefully in our next version you'll see this improve.

bdresser commented 6 years ago

@jchittoda are you still experiencing this issue?

jchittoda commented 6 years ago

@bdresser Yes, recently I left Kovan network running in Chrome for some time and it started taking more memory. Ultimately I had to kill the MetaMask from Chrome.

danfinlay commented 6 years ago

Soon we are replacing our entire provider stack, so it's possible that major change fixes this issue (or replaces it with a different one), so I think we should wait for this PR to be deployed before trying to address this:

https://github.com/MetaMask/metamask-extension/pull/4279

kumavis commented 6 years ago

wow thats a lot of memory. thanks for the report!

kumavis commented 6 years ago

also TIL chrome task manager!

sydbarrett74 commented 6 years ago

@danfinlay I'm exhibiting high memory use, as well. Should I submit a separate ticket since I'm on Win10 1803?

gitcoinbot commented 6 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

This issue now has a funding of 0.4 ETH (185.14 USD @ $462.85/ETH) attached to it.

If you would like to work on this issue you can 'start work' on the Gitcoin Issue Details page.
- Want to chip in? Add your own contribution here.
- Questions? Checkout Gitcoin Help or the Gitcoin Slack
- $30,093.05 more funded OSS Work available on the Gitcoin Issue Explorer

gitcoinbot commented 6 years ago

@lastmjs Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

[x] warning (3 days)
[ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

vs77bb commented 6 years ago

Hi @lastmjs how is this going?

lastmjs commented 6 years ago

I'll be starting to look into this today

On Mon, Jul 30, 2018, 11:28 AM Vivek Singh notifications@github.com wrote:

Hi @lastmjs https://github.com/lastmjs how is this going?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/MetaMask/metamask-extension/issues/4074#issuecomment-408944657, or mute the thread https://github.com/notifications/unsubscribe-auth/AGrSj_CuqciUWGswzdum3u1QQj3SVnixks5uL0JYgaJpZM4TiwKy .

lastmjs commented 6 years ago

I'm sorry, assessing my situation yesterday and today, I don't realistically have the bandwidth right now...good luck

gitcoinbot commented 6 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work has been started.

These users each claimed they can complete the work by 11 months, 3 weeks from now. Please review their action plans below:

pranay01 has started work.

What steps will you take to complete this task?

Example: I'd start by implementing Netflify, then I'd make mockups for the /blog page and individual post pages and proceed with building them after getting some feedback from the team. I will start work on this right away and have it ready by the end of the weekend.djlksjdasjd;lkjasldk;jsa j;adsdskjas dd;ajsdkjas;djakl;sd sdjsla;djs

Learn more on the Gitcoin Issue Details page.

gitcoinbot commented 6 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work has been started.

These users each claimed they can complete the work by 4 months, 2 weeks from now. Please review their action plans below:

1) connorchristie has started work.

I have started looking into the possible causes and found some initial concerning messages about "MaxListenersExceededWarning" which I followed and lead me to the publicConfigStore event emitter. Where I plan to determine if this is the actual root cause

Learn more on the Gitcoin Issue Details page.

ConnorChristie commented 6 years ago

After looking more in-depth into the memory usage of the extension, I see a new event listener is added for every tab opened and multiplexed into the publicConfigStore event emitter.

Below is a memory snapshot of the extension after opening many tabs and closing them right away where we see publicConfigStore._events taking up the majority of the heap. When querying the number of items in the event handler array, there were roughly 130 where the limit it initially set to 11.

The method setupUntrustedCommunication gets called whenever a page is created and metamask connects to it which ends up calling the below method which adds a new event handler to the config event emitter through obs-store

https://github.com/MetaMask/metamask-extension/blob/develop/app/scripts/metamask-controller.js#L1282-L1290

This is my initial analysis however it looks promising due to the fact the added event handlers are never removed and they add up over time after numerous tab opens and closes.

ConnorChristie commented 6 years ago

Another memory leak was found in the main controller's setupControllerConnection where it would subscribe to the on('update') event emitter. Upon opening the extension window, this would add a new handler but not remove it when the window was closed.

bdresser commented 6 years ago

fyi @kumavis @danfinlay

gitcoinbot commented 6 years ago

@ConnorChristie Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

[x] warning (3 days)
[ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

ConnorChristie commented 6 years ago

Replying for the sake of Gitcoin. - I have been able to test my changes however they require a change to ‘obs-store’ which I have a PR out for before I can submit a pull request.

It seems the major memory leaks have been fixed in previous PRs but there are still some minor ones out there which I have found and fixed

gitcoinbot commented 6 years ago

@ConnorChristie Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

[x] warning (3 days)
[ ] escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

vs77bb commented 6 years ago

@bdresser this one looks good for payout, yes? Just FYI -- even though @ConnorChristie hasn't submitted work yet, you can payout using the 'Advanced Payout' feature 👍

gitcoinbot commented 6 years ago

⚡️ A tip worth 0.80000 ETH (145.62 USD @ $182.03/ETH) has been granted to @ConnorChristie for this issue from @bdresser. ⚡️

Nice work @ConnorChristie! Your tip has automatically been deposited in the ETH address we have on file.

$32172.61 in Funded OSS Work Available at: https://gitcoin.co/explorer
Incentivize contributions to your repo: Send a Tip or Fund a PR
No Email? Get help on the Gitcoin Slack

gitcoinbot commented 6 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work for 0.8 ETH (145.62 USD @ $182.03/ETH) has been submitted by:

@ConnorChristie

@bdresser please take a look at the submitted work:

PR by @ConnorChristie

Learn more on the Gitcoin Issue Details page
Want to chip in? Add your own contribution here.
Questions? Checkout Gitcoin Help or the Gitcoin Slack
$32,318.23 more funded OSS Work available on the Gitcoin Issue Explorer

bdresser commented 6 years ago

Thanks @ConnorChristie! Paid.

Leaving this issue open. We'll follow up a couple weeks after next release to check that reports via github & support have subsided.

In the meantime, if others find memory leaks in production, let us know!

ghost commented 5 years ago

Leaving this issue open. We'll follow up a couple weeks after next release

@bdresser , 3months past, should be fine to close I guess.

hatgit commented 5 years ago

What a great bounty this was! I am chiming in here simply to note that the bounty is still open on Gitcoin and as it was marked closed here by @bdresser 29 days ago, I will ping a @gitcoinbot team member (@ryan-shea) so the Gitcoin page is updated. Cheers!

gitcoinbot commented 5 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

This Bounty has been completed.

Additional Tips for this Bounty:

bdresser tipped 0.8000 ETH worth 85.12 USD to ConnorChristie.

Learn more on the Gitcoin Issue Details page
Questions? Checkout Gitcoin Help or the Gitcoin Slack
$53,237.11 more funded OSS Work available on the Gitcoin Issue Explorer

MetaMask / metamask-extension