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

fix(core): fix memory leak on application reference destroy #22238

Closed
wants to merge 1 commit into from
Closed

fix(core): fix memory leak on application reference destroy #22238

wants to merge 1 commit into from

Conversation

tmair
Copy link
Contributor

@tmair tmair commented Feb 15, 2018

When an application reference is destroyed it stays in memory because it is still registered in the testability registry.

Closes #22106, #13725

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

When an application is destroyed the application still remains in memory because the testability registry still holds a reference to the application.

Issue Number: #22106, #13725

What is the new behavior?

When an application is destroyed it is also removed from the testability registry.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

I have created a simple example application at https://github.com/tmair/angular-app-destroy
There are two commits, one using the current version of angular and the other one using the fixed version of angular

When an application reference is destroyed it stays in memory because it is still registered in the testability registry.

Closes #22106, #13725
@vicb vicb added area: core Issues related to the framework runtime action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Feb 15, 2018
@vicb
Copy link
Contributor

vicb commented Feb 15, 2018

/ref #14819

@BenevidesLecontes
Copy link

I can confirm that this fix work. I was in trouble with memory leaks on IE, tried this fix and i saw performance increase.

@roni-frantchi
Copy link

This fix worked for me as well.
We saw a very clear and stable 20% improvement in execution time of our very (very) large test bed.

@wbhob
Copy link

wbhob commented Jul 7, 2018

Any update on this PR? It's been a few months but this looks like a good fix.

@abogartz
Copy link

Any update?

@ccchrisc
Copy link

How exactly do I test this? How do I rebuild my source with the updated angular file?

@allanalvarez
Copy link

Hi @PatNumainville
I'm trying to solve my issue about my app is 5x slower on IE compared to Chrome and Firefox. We all know IE sucks but I'm trying to optimize things as much as I can.

I'm using Angular 5 and I saw this PR. I was trying to find the packages/core/src/application_ref.ts but I could'not find it. Is this applicable in specific Angular version?

Thanks

@jasonaden jasonaden added this to the needsTriage milestone Jan 29, 2019
@mikaelboff
Copy link

Any updates on this?

@mhevery
Copy link
Contributor

mhevery commented Dec 4, 2020

already fixed in #39876

@mhevery mhevery closed this Dec 4, 2020
@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 Jan 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: review The PR is still awaiting reviews from at least one requested reviewer area: core Issues related to the framework runtime cla: yes effort1: hours risk: low target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Application stays in memory even after being destroyed