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

fix($compile): Major Memory Leak in Directive '&' attribute #6807

Closed
wants to merge 3 commits into from

Conversation

btesser
Copy link
Contributor

@btesser btesser commented Mar 23, 2014

Request Type: bug

How to reproduce: http://plnkr.co/edit/TW2TjC8JkxlDyQRCLcRg
Start on Home, click link for Leak directive, click link for Home page.
In chrome timeline force garbage collection.
Take heap snapshot. Despite being entirely gone from the page and no external references, directive is not GC'ed and because it contains a reference to it's parent, the parent is not collected either.

Component(s): $compile

Impact: large

Complexity: small

This issue is related to: a memory leak

Detailed Description:

When a directive is written using '&' reverse one way binding there is a major memory leak.
when & attribute removed, leak is gone.

Other Comments:

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.
Backport of fix on 1.3.x

Closes #6794

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.
Backport of fix on 1.3.x

Closes angular#6794
@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 (#6807)

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!

@btesser btesser changed the title fix($compile): fix memory leak Major Memory Leak in Directive '&' attribute Mar 23, 2014
btesser added 2 commits March 22, 2014 23:03
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.
Backport of fix on 1.3.x

Closes angular#6794
@btesser btesser changed the title Major Memory Leak in Directive '&' attribute fix($compile): Major Memory Leak in Directive '&' attribute Mar 23, 2014
@btesser btesser added cla: yes and removed cla: no labels Mar 23, 2014
@doodeec
Copy link
Contributor

doodeec commented Mar 23, 2014

It closes also #6801 which fixes exacty the same problem, but this one with comments looks a bit nicer

@lefos987 lefos987 added this to the 1.3.0-beta.3 milestone Mar 23, 2014
@btesser
Copy link
Contributor Author

btesser commented Mar 23, 2014

@doodeec This is for the 1.2.x branch. #6801 is for Master Branch

@lefos987 lefos987 removed this from the 1.3.0-beta.3 milestone Mar 23, 2014
@doodeec
Copy link
Contributor

doodeec commented Mar 24, 2014

@btesser Sorry, didn't notice that

@IgorMinar IgorMinar self-assigned this Mar 24, 2014
@IgorMinar IgorMinar added this to the 1.3.0-beta.4 milestone Mar 24, 2014
@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
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.

5 participants