bitwarden / clients

Bitwarden client apps (web, browser extension, desktop, and cli).
https://bitwarden.com
Other
9.28k stars 1.25k forks source link

Chrome extension memory usage keep growing when frequent use #2249

Open cloverzrg opened 2 years ago

cloverzrg commented 2 years ago

Steps To Reproduce

In chrome, every time you click the icon to open the bitwarden extension, the memory usage will increase by about 10M. When the browser is started, the extension's memory usage is only 44M. Click the icon to open the extension and close, repeat it in many, the memory usage will grow to 1G

  1. launch chrome
  2. open task manager and notice the bitwarden extension memory usage
  3. [repeat]click the icon to open and then close bitwarden extension in different tab or in the same tab
  4. close all page tab
  5. the memory usage still high

Expected Result

1.when close all tab, the memory usage should down 2.the memory usage should not increase when repeat open and close the extension in same tab

Actual Result

1.when close all tab, the memory usage still high 2.the memory usage should increase about 10M each open and close the extension in same tab

Screenshots or Videos

No response

Additional Context

No response

Operating System

macOS

Operating System Version

No response

Web Browser

Chrome

Browser Version

96.0.4664.110 (x86_64)

Build Version

1.55.0

ghost commented 2 years ago

I have same problem. This extension using so much RAM.

djsmith85 commented 2 years ago

I wanted to report back on this and say that it's actively being investigated. The same goes for the issues https://github.com/bitwarden/browser/issues/2198 and https://github.com/bitwarden/browser/issues/1770

qwx commented 2 years ago

so i try some debug function and found every time open the window, the bitwarden try to load 10M+ data. I don't have a lot of passwords save in my vault, so i think the data may related to weak password check.(maybe lage vault cause large memory leak) I really dont know about gc detail in chrome, but the timeline shows that the js heap is growing about 14M everytime when i click the icon,but gc only collect 4M memory. other 10M memory never destory after the extension windows close. Even i force chrome run gc, the data still in memory. in the heap view i guess this is weak password image in the timeline image ps. i think nodes is growing too , not only the js heap. @djsmith85 perhaps this would help, sorry about my english writing. maybe not only weak passwords, I will contuine do some research about gc in chrome.

monolithonadmin commented 2 years ago

Same issue on chromebook. 0Screenshot 2022-02-19 00 53 51

samatcolumn commented 2 years ago

I am seeing the Bitwarden Chrome extension take at least 450MB at all times and often up to 950MB of memory unless I forcibly kill it. IMO both of those are too high, and it's really putting a strain on my 8GB RAM MacBook.

kspearrin commented 2 years ago

Seems related to and https://github.com/bitwarden/browser/issues/1770

I am able to reproduce the issue on Windows, however, I am also able to reproduce the issue with other extensions that I also have installed that also have a popup window like ours (I tested with uBlock origin). Can you test this? I am not sure what level of memory management capability we have here.

samatcolumn commented 2 years ago

@kspearrin yes I agree it's related to #1770 however I do not think this is something that happens to other extensions. Here's what I see in the Chrome Task Manager: Screen Shot 2022-03-17 at 4 50 41 PM

The only other extension over 100MB is Loom which is quite advanced (and has a popup). Privacy Badger is very similar to Bitwarden (has a popup, accesses every page, icon updates as I browse) and it only uses ~60MB of RAM.

So I think it's safe to say Bitwarden has a real leak.

kspearrin commented 2 years ago

@samatcolumn As before, I can reproduce the same problem with privacy badger extension in chrome. All you have to do is open and close the popup a bunch of times. The memory footprint increases every time, just like Bitwarden. The difference is likely that you are opening and closing the Bitwarden popup much more often than the other extensions under normal use cases.

image

pratikabu commented 2 years ago

I've same problem and was about to raise a ticket. I just killed bitwarden as it was using 1 gb memory.

ghost commented 2 years ago

Same, It consumes 1-2 Gb memory. Without doing anything: image

monolithonadmin commented 2 years ago

I can see that there is a new version of the extension, but the issue still occurs.

ruzrobert commented 2 years ago

Latest Chrome, getting 2.5 GB of memory usage 😬 image

ichramm commented 2 years ago

Can confirm it tops 1 GB in Brave.

For those who don't want to restart the browser, disabling/enabling the extension will reset it (you will have to input your master key tough).

cloverzrg commented 2 years ago

Can confirm it tops 1 GB in Brave.

For those who don't want to restart the browser, disabling/enabling the extension will reset it (you will have to input your master key tough).

you can just kill the ext

ghost commented 2 years ago

Can confirm the same on Chrome/Chromium. The initial memory usage itself is in the range of 150 to 250 megs. I don't think this is a sane behaviour of a browser extension at all. When I sort memory footprint in descending order, BitWarden extension has always been the first place.

qwx commented 2 years ago

So after reading code, I found that there maybe a memory leak in popup. In popup/app.component.ts ,function ngOnInit The code

    this.stateService.activeAccount.subscribe((userId) => {
      this.activeUserId = userId;
    });

Add an observers to activeAccount (BehaviorSubject) and never unsubscribe. After click the icon so many times, the observers is increasing and never fall back. So everytime popup loaded, all memory in popup will never been freed,include weak password and user data. Unfortunately, popup using zxcvbn to detect weak password, so every memory leak will use 10MB memory at least. I dont kown it is correct or not,so please help figure this out. this image can show that the observers is incleasing when click icon: image

I don't know if it's polite to using @ in issue so i remove this line, sorry. Sorry about my english writing.

update: after some debuging, maybe there is another code also lead memory leak: When the MatchMedia is destroyed, the listeners themselves are not currently being removed. https://github.com/bitwarden/clients/blob/master/apps/browser/src/services/browserPlatformUtils.service.ts#L363-L367 maybe need unregister media query listeners on destory.

ghost commented 2 years ago

@qwx do you mean this?

qwx commented 2 years ago

@qwx do you mean this?

no, the code is in clients. https://github.com/bitwarden/clients/blob/master/apps/browser/src/popup/app.component.ts#L49-#L51

qwx commented 2 years ago

After trying some localhost debug, if i remove subscribe and MatchMedia listeners, memory stop growing. But It's a huge problem for me to fix the bug and make a pull request. I dont know how to fix the problem in the right way. I know the code is useful :( So i hope someone can do some research and fix the bug I found, and if the memory leak is still there, I will try another way to find the memory leak point.

ghost commented 2 years ago

After trying some localhost debug, if i remove subscribe and MatchMedia listeners, memory stop growing. But It's a huge problem for me to fix the bug and make a pull request. I dont know how to fix the problem in the right way. I know the code is useful :( So i hope someone can do some research and fix the bug I found, and if the memory leak is still there, I will try another way to find the memory leak point.

Just provide your workaround here and devs will take care of the rest.

qwx commented 2 years ago

After trying some localhost debug, if i remove subscribe and MatchMedia listeners, memory stop growing. But It's a huge problem for me to fix the bug and make a pull request. I dont know how to fix the problem in the right way. I know the code is useful :( So i hope someone can do some research and fix the bug I found, and if the memory leak is still there, I will try another way to find the memory leak point.

Just provide your workaround here and devs will take care of the rest.

Already provided previously~

addisonbeck commented 2 years ago

Thank you for all the input & patience folks. Proper subscription management is definitely an issue here, and definitely creating a memory leak. I'm going to be taking a look at the subscriptions across clients next week (including the work already done by @qwx, thank you) and getting those handled properly. We aren't completely sure subscription management is the only issue (this issue predates the code examples seen so far, especially StateService), but there is more work planned in this area that I will elaborate on afterwards if the subscription cleanup doesn't alleviate everything.

UPDATE 06/16/22: It might be next week until I have a PR up for this. It has ended up taking a good bit of thought and refactoring to remove the event listener for system themes in a not hacky way. I've got working builds that do it though! More info soon :)

qwx commented 2 years ago

Thank you for all the input & patience folks. Proper subscription management is definitely an issue here, and definitely creating a memory leak. I'm going to be taking a look at the subscriptions across clients next week (including the work already done by @qwx, thank you) and getting those handled properly. We aren't completely sure subscription management is the only issue (this issue predates the code examples seen so far, especially StateService), but there is more work planned in this area that I will elaborate on afterwards if the subscription cleanup doesn't alleviate everything.

MatchMedia listener also lead memory leak, https://github.com/bitwarden/clients/blob/master/apps/browser/src/services/browserPlatformUtils.service.ts#L363-L367 But i think this is easy to fix, thank you so much!

samatcolumn commented 2 years ago

@addisonbeck thanks for validating our concerns, I appreciate your positive attitude about maintaining this extension!

addisonbeck commented 2 years ago

@qwx if you're still available with a local environment I'd be interested to know how the branch above works on your machine. The media match leak should be stopped, and I'm not seeing memory growing on my own machine anymore.

I don't think the stateService subscription is relevant to this. It's not manually unsubscribed from, but AppComponent is destroyed each time the popup is closed (or at least it should be) so that subscription should be destroyed along with it. This is where we'll look next, regardless.

qwx commented 2 years ago

@qwx if you're still available with a local environment I'd be interested to know how the branch above works on your machine. The media match leak should be stopped, and I'm not seeing memory growing on my own machine anymore.

I don't think the stateService subscription is relevant to this. It's not manually unsubscribed from, but AppComponent is destroyed each time the popup is closed (or at least it should be) so that subscription should be destroyed along with it. This is where we'll look next, regardless.

I dont have local environment but I use dev tools and edit file in plugin file folder. But I will try to do some test on your branch, thank you very much. about stateService, I just google and some post said BehaviorSubject need unsubscript, and in background, observers in activeAccount are incleasing and wont destory even if the popup closed, so I think this also lead memory leak. Maybe this is why AppComponent destroyed but why memory leak? Thank you again about your work!

addisonbeck commented 2 years ago

The patch for the event listener and the patch to unsubscribe from StateService.activeAccount in AppComponent will be included in our release in August. Hopefully those two fixes gets most existing issues resolved. I'll check back after the August release and see how folks on this thread are feeling. Thanks again for all the patience and feedback!

jasonkylejacobs commented 2 years ago

Updated my Chrome extension to version 2022.8.0, but the issue still seems to be there. Everytime I open the extension, the memory usage increases, and doesn't get released at any point.

qwx commented 2 years ago

Yes, I did some test, the code in apps/browser/src/popup/app.component.ts function ngOnDestroy not working, maybe the code in ngOnDestroy never running? Or maybe we can unsubscribe all old observers when popup show?

The Theming service works perfect, event listener doesn't create memory leak now. Thanks for your great work! @addisonbeck

after walking through this issue https://github.com/bitwarden/clients/issues/3166 I wondering if there is a memory leak,all passwords will left in memory all the time until you close your browser? @djsmith85 maybe this information can help, thank you a lot. after unlock my vault 3 times ,I can find 3 master password in memory.

collinlove commented 2 years ago

Updated my Chrome extension to version 2022.8.0, but the issue still seems to be there. Everytime I open the extension, the memory usage increases, and doesn't get released at any point.

I'm using the Safari extension on the latest version of Monterey. Just updated to 8.1 and this issue is still there. :( Did it fix it in Chrome?

fuomag9 commented 2 years ago

Referencing my other response, this is still not fixed in safari https://github.com/bitwarden/clients/issues/1770#issuecomment-1217783178

crazyserb commented 2 years ago

Any update on this?

I'm on v2022.9.1 (released on Sep 12th, if I remember correctly) and am still experiencing this on Chrome.

yesitisme commented 2 months ago

is there any progress on this? I noticed this issue on Firefox and on chrome but Firefox is much worse then chrome.

the interesting thing is that I only have this issue on one device multiple browsers. I use it on many devices without an issue 2024-08-29 12_37_51-Task Manager

shawngmc commented 1 month ago

I'm also not convinced this issue is actually fixed. While I do have a lot of tabs (100+ at current count) and the browser has been open for a little while, 4GB seems... excessive.

image

I tried doing an Inspect > Memory > Take snaphot, but then in trying to load the snapshot, either the background worker process crashed or the Bitwarden window timed out and closed, taking the browser tools window with it.

KlutzyBubbles commented 1 month ago

Is there any reason this was closed? I can't immediately find anything linked that suggests it was fixed and i too am still having the issue of ram usage just going up and up sometimes up to 2-3GB.

cloverzrg commented 3 weeks ago

Is there any reason this was closed? I can't immediately find anything linked that suggests it was fixed and i too am still having the issue of ram usage just going up and up sometimes up to 2-3GB.

This issue has not been discussed for a long time, it may not be the same problem, please post a new issue, and describe the 'Steps To Reproduce'. if is a same problem, i can reopen the issue

KlutzyBubbles commented 3 weeks ago

Is there any reason this was closed? I can't immediately find anything linked that suggests it was fixed and i too am still having the issue of ram usage just going up and up sometimes up to 2-3GB.

This issue has not been discussed for a long time, it may not be the same problem, please post a new issue, and describe the 'Steps To Reproduce'. if is a same problem, i can reopen the issue

My steps to reproduce are very similar to this issue hence why i commented instead of opening a new issue.

'[repeat]click the icon to open and then close bitwarden extension in different tab or in the same tab'

Still increases ram usage with no end in sight, until i restart chrome or kill the extension.

If you think it still warrants a new issue i will do, however i am still a bit confused as to where exactly this issue was solved other than being old (which seems to only indicate that it hasn't been addressed and is still an issue).

vmirage commented 2 weeks ago

Locking the vault seems to shrink the memory usage to a normal state