Browser Memory isn't released when destroying components

[rmullins9]

I'm submitting a ... (check one with "x")

[X] bug report => search github for a similar issue or PR before submitting
[ ] feature request
[ ] support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior

When triggering ComponentRef.destroy(), no browser memory is ever released. Any injected dependencies are all still there and running, even though the component is gone.

Also:
When bootstrapping an app within an iframe, destroying that app and removing the iframe from the DOM does not release browser memory.

In both of these scenarios, browser memory builds up over time and the browser tab and angular app lock up or crash. Injected dependencies are created, but never removed.

Expected behavior

Browser memory should be released as apps and components are destroyed.

Minimal reproduction of the problem with instructions

STEPS TO REPRODUCE

REPRODUCING DEPENDENCY LEAK

1. Run the Tour of Heroes plnkr in Chrome or IE
   https://plnkr.co/edit/R9axXVtjep5VO8hWygpA?p=preview

2. Open the js console and watch the logging in the hero service, which does not stop after destroying the app.

• Note, this is the Tour of Heroes example, except that after the app is bootstrapped in main.ts, I've added a 15sec timer that runs ref.destroy(). Additionally, in the hero.service.ts file, I added a console log in the constructor that prints every second. You can see that the service continues to run even after triggering .destroy() on the component ref.

REPRODUCING IFRAME LEAK

1. Run the Tour of Heroes live example in IE 11
   https://angular.io/resources/live-examples/toh-6/ts/eplnkr.html

2. Open F12 dev tools, click on Memory and click "Start profiling to begin a performance session"

3. Make changes to the files by adding line breaks. Every time the app reloads in the iframe the memory will grow. After about 15 minutes of this the app will lock up due to the IE tab hitting the 1.5Gb memory limit. If you test with app that has more dependencies, it will lock up much faster.

This can be reproduced in Chrome too, it just takes longer for the memory leak to build up that much.

What is the motivation / use case for changing the behavior?

Large-scale angular apps that use iframes or rely on destroying components will leak memory over time and eventually crash or lock up.

Please tell us about your environment:

Mac OS 10.11.6

• Angular version: 2.0.X

Angular v2.4.1

• Browser: [all | Chrome XX | Firefox XX | IE XX | Safari XX | Mobile Chrome XX | Android X.X Web Browser | iOS XX Safari | iOS XX UIWebView | iOS XX WKWebView ]

all

• Language: [all | TypeScript X.X | ES6/7 | ES5]
  all
• Node (for AoT issues): node --version =

[wKoza]

For information, there is a bug in IE related here : https://connect.microsoft.com/IE/Feedback/Details/1742276

I had this problem at my work.

[DzmitryShylovich]

@rmullins9 can u pls add a github repo with reproduction? Plunkr contains a lot of additional script besides the angular app.

[rmullins9]

@wKoza - Yes I think it's possible that IE bug plays a role in the iframe issue. It still looks like angular isn't cleaning up the injector or dependencies on destroy() though. @DzmitryShylovich - i'll get something together shortly and post back - I still think your original post might be right, that it's related to the router also not being destroyed. Thank you both!

[DzmitryShylovich]

@rmullins9 can you try it out in a prod mode?

[rmullins9]

Hey @DzmitryShylovich - sorry for the delay. Yes I tried in prod mode with the same results. I did more research on this an put together quick demo repo based partially on @talsi 's (https://github.com/talsi/ng2-bug-reproduce-destroy)
https://github.com/rmullins9/angular2-service-destruction

Basically what I found is that if a service class has any properties, the memory allocated for those variables will never be released, even after calling destroy() on the component ref and platform ref. When the GC runs, none of that memory is released either.

Take the HeroTaxReturnService for example:
https://angular.io/docs/ts/latest/guide/hierarchical-dependency-injection.html
@Injectable()
export class HeroTaxReturnService {
private currentTaxReturn: HeroTaxReturn;
private originalTaxReturn: HeroTaxReturn;
constructor(private heroService: HeroesService) { }

.. Any memory that the browser allocates for the currentTaxReturn and originalTaxReturn variables will never be released by the garbage collector at any point until the browser tab is closed.

[mcgraphix]

After destroying the app and platform, its seems that there are references to the AppModule stored on the window. From what I can tell (and I'm not expert in tracking down memory leaks) there seem to be some methods related to Testability added to the window. I'm curious if those are storing handles to the application modules which might prevent the memory from being freed. Either way, this is still a huge problem because I need to be able to load several apps into a portal on demand so multiple angular apps need to be bootstrapped into the same page and right now, this issue prevents that from being feasible.

[DzmitryShylovich]

its seems that there are references to the AppModule stored on the window

even with enabled prod mode? have you tried 4.0?

[mcgraphix]

I tried with enableProdMode. I have not tried version 4 yet though. I'm using angular cli. Is there a way to make that use version 4 yet?

[DzmitryShylovich]

you can generate a new project using --ng4 flag or you can just update dependencies and typescript version in package.json

[mcgraphix]

ok. I'll try that. Its EOD for me here so I will have to try it later tonight or in the morning.

[mcgraphix]

Ok. I tried with v 4.0.0 RC 1 and I see the same issue. I uploaded the bare-bones app at https://github.com/mcgraphix/angular-destroy-bug
If you run that application using ng serve, and click the destroy button it will call destroy() on both the platform returned from platformBrowserDynamic() and destroy() on the NgModuleRef that which the bootstrap promise resolves with. After calling destroy the app goes away and the app-root node is removed from the dom. However, calling window.getAllAngularRootElements() returns a reference to it. So I would hypothesize that the Testability API is causing memory leaks by not releasing its references. Let alone that the Testability seems to add methods on window. I haven't tested it, but based on the code it looks like if you were to load two apps on the page, the second one would overwrite the first one. And in no case are those methods ever removed which seems bad IMHO.

[DzmitryShylovich]

@mcgraphix yeah, you are right. I'll submit a fix.

[DzmitryShylovich]

calling window.getAllAngularRootElements() returns a reference to it. So I would hypothesize that the Testability API is causing memory leaks by not releasing its references.

I fixed this.

you were to load two apps on the page, the second one would overwrite the first one. And in no case are those methods ever removed which seems bad

but this is slightly harder to fix. Need to introduce some kind of app id.

[mcgraphix]

Thanks for taking a look so quickly. Is there a way via some provider to turn off the whole testability registration? I see that there is _NoopTestabilityGetter which doesn't look like it adds anything to the window. How would one go about using that? If that could be done, at least it would be a work around in production for now.

[DzmitryShylovich]

if enableProdMode doesn't disable it then no

[mcgraphix]

It doesn't. It looks like BrowserGetTestability has a static init() method that creates an instance of the BrowserGetTestability and sets that using setTestabilityGetter(). That init method gets called from initDomAdapter() method from browser.ts. I would expect that enableProdMode would be the right place to turn that off. I may try to just hack the compiled JS file to not call that init method to see if that solves the issue. Though it may not have any effect without your fix for the TestabilityRegistry. Forgive me... I am new to this kind of thing, but is there a way I can merge your change and rebuild Angular 4 locally to test your fix without waiting for it be merged for real?

[DzmitryShylovich]

I think it will be easier to change already compiled js file inside node modules if you need it asap. it's just a 1 LOC

[mcgraphix]

I can try that. Is it just a matter of adding this:

TestabilityRegistry.prototype.ngOnDestroy = function() {this._applications.clear();};

[mcgraphix]

I did a little more digging on this last night and this morning. It seems that ngOnDestroy of the TestabilityRegistry doesn't get called when calling destroy() on the NgModuleRef or the platform. So I make a couple of "hacks". First I commented out the contents of the setTestabilityGetter function such that the BrowserGetTestability is not used. second, in the addToWindow of the NoopGetTestability, I store it on the window so that I can manually access it to call ngOnDestory. Calling it manually (and then removing the reference to it on the window) helps reduce the memory usage. You can see after calling that that there are no references to AppModule when you take a JS heap snapshot. However, even after that, you can still see references to ApplicationRef, NgControl, and AppView in both the window.webpackJsonp and window.assert. If you delete both of those off the window the heap snapshot no longer has any references to any of those object and memory is reduced by about 4 megabytes.

[DzmitryShylovich]

@mcgraphix can you add the plunkr that reproduces the issue?

[mcgraphix]

sure. I not sure how much Plunkr's own memory usage will cloud the issue though.

[mcgraphix]

Here is plunkr showing the issue as it stands in 2.x. https://plnkr.co/edit/2O7Tzrt14b6LxRhooeuK?p=preview

If you press the Log Info button it will output to the console the result of calling getAllAngularRootElements() and getAngularTestability(getAllAngularRootElements()[0]. Clicking the destroy button destroys both the NgModuleRef and the platform as I stated above. Once you to that, clicking the Log Info button will still return references correctly. If you take a JS Heap Snapshot you will see that there are still references to AppModule, AppModuleInjector, AppView, NgControl, NgZone, NgModuleResolver, etc. Some of this might be expected since you could technically want to bootstrap the app again so those might be ok. However, there are a lot of detached DOM nodes

I don't know how to get plunkr to use v4 though.

However I was able to "hack" a solution to all the issues by doing several things in my local build. Firstly, I concatenate all the JS bundle files into one JS file and wrap all of it into a closure. I add a MutationObserver on the node that the app-root is in so I can know when that node is removed. In my case, that happens when the portal navigates to another application. In the handler for that observer, I call destroy on the NgModuleRef, the platform and the router. I then delete window.assert and window.webpackJsonp. I then need to manually call the ngOnDestroy method of the TestabilityRegistry because it doesn't seem to get called automatically. I also, had to hack in some code so that the BrowserGetTestability isn't used and the Noop one is.

With all of that I am able to reclaim almost all of the memory of the application. I'm sure my case is not the core use case for Angular so I'm not sure how urgent this is for others, but for us at ADP, this is make or break issue.

[DzmitryShylovich]

I don't know how to get plunkr to use v4 though.

update config.js http://plnkr.co/edit/lU32vBJY9KYXZUTW7GQQ?p=preview

[DzmitryShylovich]

@mcgraphix you can try this fix #14819

[Goldflow]

I think somehow this issue flew under the radar of the Angular team since they haven't responded to this for a very long time... I would say try to bring it under attention where you can to hopefully get this fixed sooner.

I've asked for extra information on angular & ngrx gitter and someone also opened a thread on this on reddit/r/angular2 but so far no official comments:

https://www.reddit.com/r/Angular2/comments/90oc2f/angular_bug_since_2016_memory_isnt_released/

[gabrielalack]

Same here. Having significant performance issues, that build up as you simply navigate around our app and thinking this may be very relevant.

[1pocketaces1]

Yes this is an issue with angular framework and has not been fixed in years. I advise you switch projects to React JS/Native!

I have been putting in hours to learn it and it's incredible. I don't doubt that React will soon be the foundation of most new internet applications.

[mlc-mlapis]

@1pocketaces1 ... lol, hours ... it sounds like really hard work.

[FDIM]

@gabrielalack I would recommend to investigate what is not released and why, it might be observables that are not cleaned up or something else keeping references. Memory tab in chrome dev tools is extremely useful and can easily show you the diff between heap snapshots and where the references are being kept.

[1pocketaces1]

@FDIM the js heap only builds up and never decreases. onDestroy() does not deallocate heap memory. Even the above mentioned solution of setting all objects to null before destroying is not a perfect fix as the memory is still allocated.

@mlc-mlapis its not too bad. It's an app with 7 pages across 4 tabs (not counting onboarding pages), aws lambda backend. Took 6 months to design/build in angular, but now that the designs and layouts are set it should take me just a couple weeks to rewrite in React Native. I can already see a big difference in performance reacting to button presses and tab switches

[mlc-mlapis]

@1pocketaces1 ... Is it a mobile app? ... finally, Angular optimized and bundled code is a bit faster than just React code. But it seems that you are talking about the different platform ... React Native ... so you probably need to compare with Angular -> NativeScript to get comparable numbers.

[1pocketaces1]

Ya its a mobile app. The performance of touches isn't a big enough difference to be important, though it is noticeable. It is the memory leak issue that this thread is about, that's why we left angular and cannot use it. With a project that has this number of pages with many components on each page, the js heap piles up and the app becomes un-usable after navigating around.

[codefactorydevelopment]

i have worked on several enterprise, big projects using angular (2-6) with alot of modules, features, even heavy dom manipulations, etc...
not only have i never experienced performance problems but all projects were blazingly fast & responsive.

not trying to advocate anything, just thinking out loud if there is something outside of angular per se causing this kind of problems. some particular pattern, lib, something...

[benelliott]

@1pocketaces1 The most likely reason is that you are not cleaning up observable subscriptions when your component is destroyed. The observable will retain a handle to any subscribers to it, so if you instantiate one component that subscribes to a particular observable a thousand times without unsubscribing then a thousand instances of that component will still be retained in memory.

[1pocketaces1]

@benelliott we did not use any observables. I know exactly what the issue was lol I pinpointed it with hours in devTools. Every object that was created remained in heap memory forever

[rmullins9]

I'll be updating my original demo app to version 6 soon to re-test this issue. Honestly we haven't seen memory leak problems in quite a while.

The biggest issue I found previously is that memory allocated to service properties was not being cleaned up, even after destroying the entire app. I believe they fixed that issue.

Still, as a best practice, we've ensured that no service holds props - it's a bad way to manage app state and will lead to other problems that are solved by ngrx.

[SudhakarSundar]

For me the problem started after using the customroutereuse strategy, the components which I wanted to reuse after navigating away from that component page and returning to the same component page, I see the memory is getting built up without releasing.

…

On Tue, Aug 7, 2018, 1:12 PM 1pocketaces1 ***@***.***> wrote: @benelliott <https://github.com/benelliott> we did not use any observables. I know exactly what the issue was lol I pinpointed it with hours in devTools. Every object that was created remained in heap memory forever — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#13725 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ANNqcFivpavDY3A9rSh5ukGsoEOyAUlGks5uOdh3gaJpZM4LYXyb> .

[trotyl]

@rmullins9 I'd suggest close this issue if you don't have this issue any more, and let others open new issue per case (e.g. particular RouteReuseStrategy usage). This issue already goes off-topic and isn't possible for tracking the problem.

[rmullins9]

I retested the issue and can't reproduce it anymore. Under the same scenario, when navigating away from the components they are destroyed and allocated memory is cleaned up.

[briancodes]

On a related note, there was an issue where the ngOnDestroy() method of services provided with useFactory was never called. Any unsubscribing in the service ngOnDestroy e.g. $observableRef.complete() would be ignored. This could lead to components and views being retained in memory. This has been fixed in V6.1 beta fc03427

[pevans360]

My issue turned out to be related to Google Maps ...
https://issuetracker.google.com/issues/35821412

[rafael-andrade]

@benelliott I have been reading some blogs about unsubscribing observables and some blogs that I read say that I don't need to unsubscribe to all observables, for example, http requests, cause they do not maintain the observable in memory after the return. Is that true ?

[benelliott]

@rafael-andrade You do not need to unsubscribe from an observable if you know that it will complete before the component is destroyed. It is long-lived observables which you need to be careful with, since they will keep a reference to the subscribed component and thus it won't be destroyed until the observable completes, which could be after a long enough time that you have instantiated thousands of instances of it.

[rafael-andrade]

@benelliott I am really trying to get into observables, what do you mean when you say "it will complete". Does HttpClient.get from angular completes before components are destroyed? How can I check this kind of things? For example, I am subscribing to events from ngx-bootstrap modals (https://valor-software.com/ngx-bootstrap/#/modals). How can I know if its observable completes or not?
Sorry if it is a dumb question and thanks for the previous answer.

[1pocketaces1]

I wanted to contribute what my issue was in case someone coming here has the same. I was using ng-lottie to put bodymovin animations in my app. This was the cause as the lottie package does not remove any animation artifacts in the ngOnDestroy, so they are locked up in heap memory

[rafael-andrade]

@briancodes I was reading the HttpClient response code, it is something like this

        if (ok) {
          // A successful response is delivered on the event stream.
          observer.next(new HttpResponse({
            body,
            headers,
            status,
            statusText,
            url: url || undefined,
          }));
          // The full body has been received and delivered, no further events
          // are possible. This request is complete.
          observer.complete();
        } else {
          // An unsuccessful request is delivered on the error channel.
          observer.error(new HttpErrorResponse({
            // The error in this case is the response body (error from the server).
            error: body,
            headers,
            status,
            statusText,
            url: url || undefined,
          }));
        }

Does that mean that when occurs an error the observable does not complete or it completes somewhere else?

[cgatian]

Please take this discussion offline.

…

On Wed, Sep 5, 2018, 8:23 AM Rafael Andrade ***@***.***> wrote: @briancodes <https://github.com/briancodes> I was reading the HttpClient response, something like this if (ok) { // A successful response is delivered on the event stream. observer.next(new HttpResponse({ body, headers, status, statusText, url: url || undefined, })); // The full body has been received and delivered, no further events // are possible. This request is complete. observer.complete(); } else { // An unsuccessful request is delivered on the error channel. observer.error(new HttpErrorResponse({ // The error in this case is the response body (error from the server). error: body, headers, status, statusText, url: url || undefined, })); } Does that mean that when occurs an error the observable does not complete? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#13725 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABq8aqYt3_4Rh2M6kUxxi8s0bpwfeq4Cks5uX8JPgaJpZM4LYXyb> .

[benelliott]

@rafael-andrade If an observable errors it has terminated and will not emit any more values, in a similar way to when it completes. They are technically different events but yes, an error event closes the stream as well.

You can read more here:
http://reactivex.io/rxjs/manual/overview.html#executing-observables

[m0uneer]

@rafael-andrade
Check my answer at SO: https://stackoverflow.com/a/51732897/3100587?stw=2

[K0N0R]

I had this problem using single-spa
I found stupid workaround to memory leak problem.
try to put this code before angular modules load, maybe it will help you

import { TestabilityRegistry } from '@angular/core';
...
TestabilityRegistry.prototype.registerApplication = () => void 0;
TestabilityRegistry.prototype.unregisterApplication = () => void 0;
TestabilityRegistry.prototype.unregisterAllApplications = () => void 0;
TestabilityRegistry.prototype.getTestability = () => null;
TestabilityRegistry.prototype.getAllTestabilities = () => [];
TestabilityRegistry.prototype.getAllRootElements = () => [];
TestabilityRegistry.prototype.findTestabilityInTree = () => null;

it just prevent TestabilityRegistry to do anything, so it wont keep reference to module after unmount

[angular-automatic-lock-bot]

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