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

Components not being garbage collected by browser, causing major memory leak #45080

Closed
kevinpbaker opened this issue Feb 14, 2022 · 21 comments
Closed
Labels
area: core Issues related to the framework runtime memory leak Issue related to a memory leak
Milestone

Comments

@kevinpbaker
Copy link

kevinpbaker commented Feb 14, 2022

Which @angular/* package(s) are the source of the bug?

Don't know / other

Is this a regression?

No

Description

Stack Overflow Post: https://stackoverflow.com/questions/71026959/memory-leak-garbage-collection-in-browser-not-collecting-components

Summary:

  • when a component gets created with *ngIf="true" or with ViewContainerRef.createComponent(componentFactory: ComponentFactory), and then destroyed, with *ngIf="false" or with ViewContainerRef.clear(), the component stays in memory, and can't be garbage collected by the browser
  • this causes a significant memory leak (especially in larger apps that use many nested components)
  • this is a tricky bug because:
    • if you download the minimal reproduction app, either a or b will happen
      a. it will leak memory right away, and components won't be garbage collected (regularly or with the manual garbage collection button provided by most browsers for debugging purposes)
      b. it won't leak memory right away, and it will happily garbage collect components regularly or with the manual garbage collection button provided by most browsers for debugging. After enough time has elapsed, or after you've restarted the app an arbitrary amount of times, it will eventually get into the memory leaking state
    • therefore, if my coworkers and I download the repo, then run the app, it seems to be a coin toss on whether or not this bug gets reproduced right away
    • if you're one of the lucky few that doesn't get it to happen right away, perhaps try it on a different computer, or wait a couple minutes and refresh the web app a couple times
  • the fact that it doesn't happen first try every time makes it hard to prove that there is a bug with this minimal reproduction app
    • I've included screen shots in the stack overflow post (near the bottom), that proves that this is indeed happening
    • it does however happen every time with a much larger production app that I'm working on

Please provide a link to a minimal reproduction of the bug

Repo: https://github.com/kevinpbaker/angular-memory-killer
S3 bucketed app: https://angular-memory-killer.s3.us-west-2.amazonaws.com/index.html

Please provide the exception or error you saw

There is no error thrown. The components in memory aren't being garbage collected, and will eventually crash the browser window.

Please provide the environment you discovered this bug in (run ng version)

I have upgraded this project to be the latest version of angular 12, and the latest version of angular 13, and it leaks in 11, 12, and 13. I have also tried it on multiple computers (M1 mac, older intel mac, windows pc, windows laptop).

Angular CLI: 11.2.18
Node: 16.14.0
OS: darwin arm64

Angular: 11.2.14
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router
Ivy Workspace: Yes

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1102.18
@angular-devkit/build-angular   0.1102.18
@angular-devkit/core            11.2.18
@angular-devkit/schematics      11.2.18
@angular/cdk                    11.2.13
@angular/cli                    11.2.18
@angular/material               11.2.13
@schematics/angular             11.2.18
@schematics/update              0.1102.18
rxjs                            6.5.5
typescript                      4.0.8

Anything else?

I've included a link to a stack overflow post I made that adds additional information and pictures to show that it is indeed leaking memory.

Microsoft Edge has an experimental feature called "Detached Elements" that can be enabled to see any objects that are dangling and ready to be garbage collected. You can use this feature to visually see that the angular components aren't being garbage collected (I've also included screen shots of this in the stack overflow post).

@pkozlowski-opensource pkozlowski-opensource added area: core Issues related to the framework runtime memory leak Issue related to a memory leak labels Feb 15, 2022
@ngbot ngbot bot modified the milestone: needsTriage Feb 15, 2022
@sod
Copy link
Contributor

sod commented Feb 16, 2022

Have you tried your reproduction in incognito without extensions? I ask as my edge & chrome browsers don't show the leaking behavior. Even after 3 minutes the app is stil at 5.5 MB heap for me. Maybe there is an extension that retains dom nodes. What do you see if you open the tree on detached html in the screen you provided https://i.stack.imgur.com/hkyw8.png, is there is hint about who holds onto the memory?

@crisbeto
Copy link
Member

I haven't tried running the app yet, but one thing that stands out is the presence of LViewDebug in the memory snapshot. This means that the app is running in development mode which includes extra debugging information. Have you tried running it in production mode?

@kevinpbaker
Copy link
Author

I am grateful that there are eyes looking at this issue. Thank you both for replying. I have attached an image of the command I used to run the memory killer app below (showing that it's running in production mode).

Screen Shot 2022-02-16 at 8 43 51 AM

I've also included a video showing me manually clicking the garbage collection button (while the app is in production mode), but the browser still can't collect the nodes, because it appears the angular components keep a reference to something that won't allow them to be collected, so they retain their size, and aren't being collected.

angular_leak.mp4

@sod the original images I took were ran on Edge without extensions, and chrome with extensions. I only downloaded Edge in order to use their nifty detached elements feature. It's hard to tell what's keeping a reference by looking at the memory inspector. I find it more confusing than helpful in this situation.

@crisbeto I've been able to get this memory killer app to work in both production and development mode. My coworkers have been able to see the memory leak on their machines as well (in both modes), but like I said above, it seems to be hit or miss on whether or not it happens first try. If it doesn't, we usually open it in a new tab (sometimes let it run for a bit), then close it, and repeat this process until it does start leaking memory. It's easier to tell if it's leaking by using Microsoft Edge and its experimental feature called "Detached Elements". If you don't, then you have to take memory snapshots and see if the DOM nodes are retained (this takes longer).

I really hope that you both can reproduce this, so that this issue isn't overlooked.

@andrewcretin
Copy link

andrewcretin commented Feb 16, 2022

Hi all. Thank you @sod and @crisbeto for looking into this. I want to provide some context to the use case that is burning us in our business' application. What @kevinpbaker has provided is a stripped down version of the app functionality to easily pinpoint the memory leak - this was a process we went though while debugging. We think it may have to do with the 'flex' attribute, but was unable to confirm this 100% (See stack overflow post with some additional details)

BudSense provides simple menu management and merchandising solutions for legal cannabis retailers. As you can see in the below video, our software will loop through 'menus' as specified by the store operator. This issue happens when there are 3 menus being looped. 1 menu in the foreground, 1 rendered in the background (so it's ready to present) and the third should be deallocated when not in position 1 or 2. This action is not happening, so every time the menus switch, a new detached node gets added, and the heap size grows.

Our clients run this Display app on a mix of devices, but primarily it can be bucketed into 2 categories:

  • Asus ChromeBit running Chromium and loading the URL directly
  • BudSenseTV Android App - this app will fetch the configuration from our API then present the URL through an embedded webview. (Also Chrome Based)

These menus run all day while the stores are open, so as you can imagine the memory leak bubbles up and the app crashes every few hours (depending on the memory available on the hardware device)

Reproducing and solving the issue on the stripped down app included will be best path forward, but I just wanted to provide some context as to where this is being used and some more information in that regard. Below are the links to the actual product menu, screen recording of the detached nodes increasing, as well as some screenshots of heap snapshots.

Menu URL: (run in portrait orientation 1080 x 1920)
https://display.mybudsense.com/#/display/f02aa9b5-e9b7-4f82-9647-a9e1b1b06ca8
Screen Recording:
https://user-images.githubusercontent.com/6923250/154299328-5abadd4c-4e92-4639-9e4e-047e647a6238.mp4
Heap Snapshot 1:
memory-leak-prod-application-heap-1
Heap Snapshot 2:
memory-leak-prod-application-heap-2

@yousafnawaz
Copy link
Contributor

I have tried my side on chrome and edge both in incognito mode and I don't see any memory leaks, tried multiple times, repeated refresh.. heap is still around 5. Have you tried performance monitor that is available on both browser that shows real-time memory usage and DOM nodes count..
image

@kevinpbaker
Copy link
Author

kevinpbaker commented Feb 16, 2022

@yousafnawaz hence why my mental health has taken a massive blow. Like I said, sometimes it does it, sometimes it doesn't.

This morning it was happening on my coworkers computer, and now when he tries it, it no longer does it.

I've tested this on a 2015 Macbook Pro with an Intel chip, a new Macbook Pro with an M1 chip, a windows desktop computer, and a windows Lenovo laptop (all with incognito mode, and regular browsing). All will sometimes leak, and sometimes not. Therefore, it is not a computer specific problem.

However, trying to prove that it is a problem has become something that keeps me up at night.

Here is a video of it leaking on both Microsoft Edge and Chrome (side by side):

angular_edge_and_chrome_leak.mp4

@yousafnawaz
Copy link
Contributor

@kevinpbaker, the link shared by @andrewcretin
https://display.mybudsense.com/#/display/f02aa9b5-e9b7-4f82-9647-a9e1b1b06ca8 is continuously increasing memory on both chrome and edge in incognito mode at my side...

@paulhewitt
Copy link

@sod @crisbeto @yousafnawaz

Environment:
Microsoft Edge v98.0.1108.55
2.2 GHz i7 MacBook Pro running macOS v10.15.7

I was able to get the app to leak memory in both prod, as well as dev, in an incognito window ensuring no add-ons. I have attached screen shots below highlighting the leaks. As expected, this took me quite a while to reproduce. It took about 20-30 minutes of constantly refreshing, dumping cache, opening new tabs, quitting edge entirely, etc. The goal now is to figure out the exact reproduction steps, as right now it seems to be completely random, which does not make any sense whatsoever.

Dev:
Screen Shot 2022-02-16 at 12 02 50 PM

Prod:
Screen Shot 2022-02-16 at 12 15 22 PM

@paulhewitt
Copy link

I was able to once again reproduce the memory leak running the Memory Killer in prod, this time on Chrome v 96.0.4664.110. Again, it seems to happen randomly, and thankfully it happened on my 4-5th refresh. I am not too sure if these will be useful, but I took some heap snapshots 10 minutes apart once the leak occurred.

The first heap snapshot can be downloaded here. At this point, the heap was roughly 13MB.

I took a second snapshot ~10 minutes later, and the heap was roughly 24MB, and still trending upwards. The link to download that one can be found here.

Again, I am not sure if these will be much use, but I am just trying to gather as much data as possible. Finally, I did a quick one minute performance recording in the inspector, and I have included a screenshot below. I tried to let it run for a couple of garbage collection cycles, so you can see the heap gradually increasing.

Screen Shot 2022-02-16 at 1 55 41 PM

@LAlves91
Copy link

Hey!

So, I might be WAY off here...I'm currently studying Angular's inner structure, trying to understand how it works on a lower level and I thought this issue would be a good way to dive deeper, even though I'm kinda out of my league here hahahhah

I was doing a few memory snapshots with the BudSense link provided, and it seems like there's a lot of self references throughout the detached elements (detached HtmlDivElements wich references themselves and other HtmlDivElements, which also do the same thing - as if they were detached but those cross-references kept them from being collected).

Couldn't it be caused some kind of component hierarchy (an abstract component extended by one or more components) with self-references through ViewChildren or other query-like features?

@crisbeto
Copy link
Member

crisbeto commented Feb 17, 2022

It's worth keeping this issue in mind #41047. The tl;dr is that we're currently patching some internal data structures onto the DOM nodes which can make existing memory leaks worse. We're in the process of landing a PR that'll change how the data structures are stored in v14 #45051.

@crisbeto
Copy link
Member

I tried running both the memory killer app and the menu app mentioned above, but I couldn't get it to leak more than 13 DOM nodes which would usually be GCed on the next snapshot. There's a recording of my timeline below.

Screen.Recording.2022-02-17.at.13.24.02.mov

Looking at your heap dump, it does have some references in __ngContext__ so it's possible that #45051 will help with it. It's hard to say before the change is actually released though. You could try changing the MemoryKillerComponent to the following which should have a similar effect:

export class MemoryKillerComponent {
  constructor(private _elementRef: ElementRef) {}

  ngOnDestroy() {
    [
      this._elementRef.nativeElement,
      ...this._elementRef.nativeElement.querySelectorAll('*'),
    ].forEach((el) => {
      el.__ngContext__ = null;
    });
  }
}

@kevinpbaker
Copy link
Author

kevinpbaker commented Feb 17, 2022

@crisbeto it looks like you haven't been able to get the app in the state where it uncontrollably retains memory. This deeply troubles me, because I can get it into this state fairly consistently.

I read through #41047 and #45051. I also tried adding the code that you specified to the memory killer app while I have it in the leaking state, and it still leaks memory. I do see this ngContext in the memory snapshot when using production mode (when the app is leaking).

Forgive me, but this ngContext gives me more questions than answers.

Questions:

  1. Is the solution above a fool proof fix for this ngContext problem? If it is, does that not mean that this is an unrelated memory leak?
  2. Did you discover if storing references to an 'LView' on most DOM nodes within ngContext or by saving 'LContext' which has a reference to the 'LView' causes intermittent memory leaks like in the simple memory killer app provided?

I've discovered when I get the memory killer app in the leaking state, that If I remove the display: flex attributes from the component classes, then the components get garbage collected properly. If I then add them back, it immediately goes back into the leaking state. When I say leaking state, I don't mean the state that you showed above, I mean the state that we've been able to get the app in, which will leak to the point of failure. This leads me to my next question:

  1. Do style attributes on DOM elements affect this 'LView' memory leak problem that you're fixing? Specifically something like the display: flex attribute?

Concerns

I'll admit that I'm worried. The memory killer app provided is the stripped down version of what we are experiencing, and the person that holds the skills and knowledge to shed more light onto this problem can't get the memory killer app provided into this failing state on their machine. That's why this issue has caused me so much grief. Why am I able to consistently get this app into a failing state on multiple machines, new and old, different OS's, and different makes and models, and other people have trouble doing so? 😞 (I re-read this and it comes across as if I'm attacking you and others. I'm not. I'm just depressed about this issue, and the fact that it's somewhat hard to reproduce).

I will continue to be as helpful and as skillful as possible in documenting this issue, so that we can eventually pinpoint exactly what the problem is.

@crisbeto
Copy link
Member

To answer the questions:

  1. The solution I provided above is a quick way to check if the __ngContext__ fix would resolve the issue in the memory killer app, but it isn't foolproof. The actual solution is in __ngContext__ magnifies native Chrome memory leaks #41047.
  2. __ngContext__ doesn't cause memory leaks by itself. If the app doesn't have leaks at all, __ngContext__ isn't going to introduce new leaks. It comes into play if the apps has some leaks which __ngContext__ will make worse.
  3. Style attributes won't be affected by the __ngContext__ issue. We have one code path where a reference to the host DOM node will be retained when there are styles associated with it. It only applies to components with encapsulation: ViewEncapsulation.ShadowDom though.

As for reproducing it, it's entirely possible that I'm not doing something in the same way as you or there's something in my environment that prevents it from happening. What I did today was to npm i, ng serve and then take memory snapshots in the dev tools. I had it running for about 15min with no change in the amount memory being consumed.

One thing worth trying is to check if the leak happens in other browsers like Firefox or Safari. It would at least help us narrow it down to a framework issue or a potential problem with a browser.

@kevinpbaker
Copy link
Author

kevinpbaker commented Feb 18, 2022

@crisbeto after a lot more testing, I think this issue is specific to Microsoft Edge and Chrome. I was unable to get Safari or Firefox to leak memory using the memory kill app provided.

Safari

The memory footprint in Safari will always climb and then plateau. Even if I spam the garbage collection button it remains at or below the plateau value. Here is an image showing this behavior:
Screen Shot 2022-02-18 at 10 52 12 AM

Firefox

The memory footprint in Firefox gets cleaned up regardless of how many times I try to get it into the leaking memory state. Here is an image showing this behavior:
Screen Shot 2022-02-18 at 10 45 52 AM


While doing this I had all 4 browsers running at the same time. All of them running the memory killer app. Microsoft Edge and Chrome would leak memory, but Safari and Firefox would not.

From the tests and information above, I think we can conclude that this isn't an Angular Framework problem, but a browser problem.

Also, one of the computers is factory new that I used to leak memory on Microsoft Edge and Chrome. I install node.js, then I clone the memory killer app using git clone https://github.com/kevinpbaker/angular-memory-killer.git, then I run npm install and ng serve --prod --port 4204. I am able to get Microsoft Edge and Chrome to leak memory doing this.

@crisbeto
Copy link
Member

crisbeto commented Feb 19, 2022

Interesting, #41047 does mention that the leaks were specific to Chrome. It may be worth checking again after next week's 14.0.0-next release which contains the __ngContext__ fix. You can also try it now by installing from https://github.com/angular/core-builds.

@crisbeto
Copy link
Member

The __ngContext__ changes have been released in version 14.0.0-next.4. Did anybody get the chance to check if it helps with the memory leak?

@kevinpbaker
Copy link
Author

kevinpbaker commented Mar 1, 2022

@crisbeto I upgraded the angular-memory-killer app to 14.0.0-next.4. Chrome and Microsoft Edge still leak memory in the same way described throughout this conversation.

I've read that Chrome and Microsoft Edge are both built on top of the same rendering engine. I think there is a bug somewhere in the shared foundation between Chrome and Microsoft Edge, specifically with what internal references are placed on DOM nodes when display: flex, display: inline-block, display: inline-flex, display: inline-grid, display: inline-table are used. These display types make the angular-memory-killer app leak memory.

If I use display: block, display: inline, display: grid on the components within the angular-memory-killer app, then it doesn't leak memory.

I feel like this information needs to be forwarded to the Chrome Dev Team, but I'm not exactly sure on how to go about doing that.

@kevinpbaker
Copy link
Author

Chromium bug: https://bugs.chromium.org/p/chromium/issues/detail?id=1308845

This issue has been fixed within the chromium browser. It hasn't been released yet, but it can be tested with Chrome's Canary browser: https://www.google.com/intl/en_ca/chrome/canary/

Broken Version 100.0.4896.60 (Official Build) (arm64)

broken.mp4

Fixed Version 102.0.4987.0 (Official Build) canary (arm64)

fixed.mp4

@AndrewKushnir
Copy link
Contributor

@kevinpbaker thanks for the update!

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators May 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: core Issues related to the framework runtime memory leak Issue related to a memory leak
Projects
None yet
Development

No branches or pull requests

9 participants