clients icon indicating copy to clipboard operation
clients copied to clipboard

Chrome extension memory usage keep growing when frequent use

Open cloverzrg opened this issue 2 years ago • 29 comments

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

cloverzrg avatar Dec 29 '21 02:12 cloverzrg

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

ghost avatar Jan 04 '22 21:01 ghost

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

djsmith85 avatar Feb 14 '22 16:02 djsmith85

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.

qwx avatar Feb 16 '22 02:02 qwx

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

monolithonadmin avatar Feb 18 '22 23:02 monolithonadmin

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.

samatcolumn avatar Mar 17 '22 12:03 samatcolumn

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.

kspearrin avatar Mar 17 '22 13:03 kspearrin

@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.

samatcolumn avatar Mar 17 '22 16:03 samatcolumn

@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

kspearrin avatar Mar 17 '22 18:03 kspearrin

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

pratikabu avatar Apr 07 '22 21:04 pratikabu

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

ghost avatar May 03 '22 09:05 ghost

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

monolithonadmin avatar May 03 '22 09:05 monolithonadmin

Latest Chrome, getting 2.5 GB of memory usage 😬 image

ruzrobert avatar May 08 '22 09:05 ruzrobert

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).

ichramm avatar May 13 '22 11:05 ichramm

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

cloverzrg avatar May 14 '22 10:05 cloverzrg

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.

ghost avatar May 22 '22 06:05 ghost

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.

qwx avatar May 26 '22 17:05 qwx

@qwx do you mean this?

ghost avatar May 27 '22 10:05 ghost

@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 avatar May 27 '22 13:05 qwx

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.

qwx avatar Jun 10 '22 10:06 qwx

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.

ghost avatar Jun 10 '22 10:06 ghost

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~

qwx avatar Jun 10 '22 14:06 qwx

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 :)

addisonbeck avatar Jun 10 '22 18:06 addisonbeck

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!

qwx avatar Jun 11 '22 09:06 qwx

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

samatcolumn avatar Jun 12 '22 09:06 samatcolumn

@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.

addisonbeck avatar Jun 21 '22 17:06 addisonbeck

@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!

qwx avatar Jun 23 '22 01:06 qwx

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!

addisonbeck avatar Jun 24 '22 13:06 addisonbeck

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.

jasonkylejacobs avatar Aug 05 '22 13:08 jasonkylejacobs

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.

qwx avatar Aug 05 '22 19:08 qwx

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?

collinlove avatar Aug 17 '22 04:08 collinlove