Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

fix($compile): fix memory leak #6801

Closed
wants to merge 1 commit into from
Closed

Conversation

lgalfaso
Copy link
Contributor

For functions that have a '&' binding on the scope, change the function from an inline definition to an external definition so the function closure has no access to the parent scope.

Closes #6794

Note: I am not sure this if this is a Chrome GC bug (as the function never uses the parent scope), but for the scenario from the issue, the patch fixes the leak

@mary-poppins
Copy link

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

  • Uses the issue template (#6801)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

For functions that have a '&' binding on the scope, change the
function from an inline definition to an external definition so
the function closure has no access to the parent scope.

Closes angular#6794
@lgalfaso
Copy link
Contributor Author

Attached is a Chrome heap snapshot after clicking on "home", then "leak", then "home". It shows the reference to the function (in fact, the entire scope was not GCed)

I still think this is a Chrome bug.

As a side note, I was not able to make it leak more than one scope

screen shot 2014-03-22 at 2 16 30 pm

@IgorMinar
Copy link
Contributor

as discussed with @lgalfaso today there is no evidence that this patch actually does anything. applying it or not, the leak is still present. the leak that we can reproduce is present even when & bindings are not being used.

@IgorMinar IgorMinar closed this Mar 26, 2014
@lgalfaso
Copy link
Contributor Author

lgalfaso commented Apr 1, 2014

Reopening for visibility

@lgalfaso lgalfaso reopened this Apr 1, 2014
@IgorMinar IgorMinar modified the milestones: 1.3.0-beta.5, 1.3.0-beta.4 Apr 2, 2014
@lgalfaso lgalfaso closed this Apr 3, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Major Memory Leak in Directives when using '&' binding
4 participants