Memory Leak in Navigation

[JimGreenberg]

Description of problem:

My team and I are working on an app and we noticed that after a number of navigations the app would ANR and crash under certain conditions. We suspected a memory leak- here is an example of the memory profile during a few repetitions of an action containing a navigation:

Note that more memory is freed after each GC.

Here is that same action repeated, but without the navigation:

Note that we're able to perform many more actions before the OS decides to GC, indicated by the fineness of the increases in allocated memory- and also that after the GC the memory drops to its starting point whereas the first example this was not the case.

Isolating the issue:

I've created a simple app here: testapp.zip
It consists of 2 pages with a large button that navigates between them. Because the components are so small compared to a production app, this did take some time, however the problem is still visible in the memory profile- memory is unbounded and increasing:

What I have tried:

• Experimented with clearHistory option with navigationExtras
• Periodically calling the V8 GC explicitly
• Used the vanilla angular router outlet instead of NS's page router outlet
• Created a custom reuse strategy (page router outlet does not play nicely at all)
• Used the vanilla locationstrategy (NSLocationStrategy still spun up however)
• Refactored to have specific navigation be a child route of another component (This mitigated the issue heavily- enough so that an ANR might never occur under normal use)
  • This of course comes at the cost of defeating all of the features NS put in the page router, so implementing this fullscale seems impractical

Conclusion:

Should components revisited via forward navigation be reinstantiated? This seems like it could be the culprit, however I cannot verify. I'm curious as to whether there are some best practices that I missed, because this seems like such a general issue I'm surprised nobody else has encountered it- everything navigation related that we use is right out of the box. Similar issues posted here are either too old to be relevant (and were already marked as resolved) or related to something else entirely (images in particular).

Thanks for reading

[jogboms]

@JimGreenberg I'm subscribed to this, this has been a teething problem. @vakrilov 👍

[mhmo91]

I ended up switching to angular router outlet instead of page router n create my own stack of navigation ((

[vakrilov]

Hey @JimGreenberg
Thanks for the detailed explanation and the sample project!

Some clarifications:
Indeed when navigating forward(deeper) in the application with page-router-outlet - all the previous pages are kept in the nav-stack (that is the native behavior and also that is the reason for the custom router reuse strategy). This means that they are not subject to GC, as a reference to them is still kept in the backstack. Making navigation with clearHistory should clear all references and ultimately let GC collect the previous pages - you will loose the history though.

Also using the stock <router-outlet> will create and destroy the components on each navigation, but as you said you will loose the native-ness of the page navigation.

So, here some suggestions on the particular problem:

1. It is a good idea to avoid having a really deeps navigation stack. So if the user will come back to a particular page many times (ex. master -> detail -> master -> detail ....) you might consider doing back() navigation instead of forward or for clearing the history whenever the user arrives to the home screen.
2. Images - they are usually eating a lot of memory. You can check on the images optimizations article in the docs for ideas of limiting their impact.

I think there is a room for improvement in the framework for supporting such scenarios. So any ideas and suggestions are welcome.
One thing that we can probably investigate further is, wether there is a way to release large images (bitmaps) of elements that are currently in the navigation stack but now showing.

[JimGreenberg]

Hi @vakrilov, thanks for the response
I had thought that using the clearHistory parameter on ExtendedNavigationExtras would at least prevent memory from growing- this wasn't the case.
Pictured are 4 similar trials of the test app- 2 with clearHistory and 2 each using routerLink and a regular navigate(). There aren't any major differences- all 4 follow the same pattern so I didn't feel the need to distinguish them.

As we would expect, the tests done with clearHistory logged that the component was being destroyed (and likewise, that without clearHistory the components are not destroyed)- but again, no drop in memory.

Using back() did work- we returned to the previous component and no memory increase was seen. But manually calling back() in order to keep the app from running out of memory seems hacky- and also for our app in particular we want to prevent the user from going back to certain pages.

The suggestion I would make is to have a configurable stack limit for the navigation- so developers could limit the navigation stack to a finite number of navigations- but this might be a much larger refactor than expected.

[vakrilov]

For sure the clearHistory parameter should do a proper cleanup and prevent memory leaks. So this one definitely sounds like a bug and we will investigate it.

About your other suggestion (limiting the nav stack) - indeed it might be more challenging thing to implement as you will have to clear only portions of the navigation stack. It is a good suggestion for a feature though, so it will be a good idea to open a separate issue about it.

[ADjenkov]

The abnormal memory behaviour is probably related to these open issues - Issue1/Issue2 in Angular repository.
Even after explicit ComponentRef.destroy() in our custom reuse strategy the memory for that component is not released.
OnDestroy is properly called, but I suspect that any injected dependencies are still there and running, even though the component is gone.

[EddyVerbruggen]

Just to add my 2ct. I recently had the pleasure to investigate a memory leak on an N-Angular app. The problem was only observable on iOS. Android released the memory just fine.

It turned out the app had a lot of rxjs subscribe() calls but it never unsubscribed because the assumption was that would not be required because the component got destroyed which would clean up the subscriptions. Not on iOS it didn’t.

So we added unsubscribe() calls in ondestroy and the problem was solved.

[ADjenkov]

Hey @EddyVerbruggen,
Most of the comments, from the Angular issues I've mentioned above, are indeed pointing to rxjs subscriptions. Can you elaborate more on the unsubscribe() inside onDestroy that you have done? There are some subscriptions you've made before that and you manually unsubscribe from them?
In the tests I've made there weren't any explicit subscriptions but I guess there could be some behind the scenes done by Angular)

[EddyVerbruggen]

Yeah in my case those were manual subscribes. Nothing automatic / behind the scene framework stuff that causes trouble afaik. So manually unsubscribe (in ondestroy) anything you’ve subscribed to previously - if using router back() navigation destroy should run and you get rid of the subscriptions.

[Zhuoqin]

Hi @JimGreenberg, has there been any new progress on this issue?
I countered exactly same problem as you shown above. It seems normally happened to most SPA, it doesn't require page refresh but it heaps up the memory storage. So we've done some research, This link helps me a bit, but our memory leak is still a big problem that even slows the app. Just wonder how far you've got this solved. Thanks.

[JimGreenberg]

Hi @lankaixi , unfortunately no- we haven't made any progress specifically on this issue. We did as much as to identify and isolate the problem, and after exhausting the things in our control (like making sure all our rxjs subscriptions were properly disposed etc) we just left it in the back of our minds. As it turned out- none of our beta testers mentioned anything nor did we encounter it in our internal testing through normal use. While I'm keen on a solution, the "problem" wasn't really pressing- so we discontinued efforts towards it. Sorry I couldn't be more help

[vakrilov]

Hi @JimGreenberg and @lankaixi
I did some more through profiling and also testes same scenarios with web-angular project (my initial intention was to be able to isolate a problem in Angular and log it properly).

Long story short: enabling angular prod mode significantly improved the situation:

// in main.ts and main.aot.ts
...
import { enableProdMode } from "@angular/core";
enableProdMode();
...

@JimGreenberg - if you enable prod mode for the distributions for your beta testers, that might explain why they have never noticed a problem.

More Details

I've tested with a modified version of your original project - I have included 1000 Sliders (which are actually native android.widget.SeekBar in android) on the first page so that I can easily see them in the profiler and see if they have been collected in memory dumps. Here is the project:
testApp.zip.

Here is what I did in the tests:

1. Land on page1 (the one with the many Sliders)
2. Perform a couple of forward navigations from page1->page2->page1->page2 ...
3. Finally do a page1->page2 navigation, but with clear history. This should release the references to all cached pages by the previous navigations
4. Force Java GC (from android profiler) wait a couple of seconds and do it again. The reason for this that even if you force the Java GC - it might not release all elements because it needs to synchronize with the V8's JavaScript GC.

Here is the result without enableProdMode():

And here is the result with enableProdMode() - significantly better:

Tips

1. Make sure you enable pod mode for production builds :)
2. You still have to be careful with rxjs subscriptions - it is still the easiest way to get a memory leak. The ngOnDestroy() hook is still called for items in the cache that are cleared on navigation with "clearHostory" - so this is still the right place to do it.