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

Leaking $scope when method on scope references $scope itself #5527

Closed
swar30 opened this issue Dec 24, 2013 · 14 comments
Closed

Leaking $scope when method on scope references $scope itself #5527

swar30 opened this issue Dec 24, 2013 · 14 comments

Comments

@swar30
Copy link

swar30 commented Dec 24, 2013

When we have a view / child scope which is added and removed to page and in that view we have a controller that adds methods to it's scope which reference $scope in body of method like

$scope.flipFlag = function(){ $scope.flag = !$scope.flag }

When view is destroyed the scope and all watchers on scope and anything they reference is leaked.

If instead you write method like

$scope.flipFlag = function(){ this.flag = !this.flag }

Then there seems to be no leak.

Might be related to #4026

Reproduction can be seen at http://plnkr.co/edit/JZbtZ13Ml1aDjNJug1gu?p=preview on top of angular 1.2.5. Originally reproduced on our application on top of angular 1.1.4.

What we do in this reproduction is set and unset ng-include to point to a template with a controller.
In the controller we have scope method that references $scope.

To see the leak, on reproduction page, after accessing it, take a heap dump and there on summary view filter by Child constructor.
You will see that there are two scopes leaking, with ids 004 and 005, where 005 is the scope for the controller injected in the view.html and 004 is it's prototype.
From what I can see 004 isn't released because it's held by 005 (as it's prototype).
And 005 isn't released, well here it's not so clear, from what I can see it's not released because it has a function that has it in it's closure / scope.

Running GC several times in chrome does not remove the leaked scope.

In our application this causes the leak of the entire UI (JS and DOM) every time we switch from one view (state) to another, our app is big enough that after a few transitions we see that tab with our app reach 1GB of memory.

More verbose explanation of the issue at https://groups.google.com/d/msg/angular/5aBiuBZCzus/vJQKk9ebVGkJ

@ghost ghost assigned tbosch Dec 28, 2013
@tbosch
Copy link
Contributor

tbosch commented Jan 2, 2014

Does not show up when:

  1. open the plunker (the url from previewing in windows mode) in a new tab
  2. open DevTools and take a heap dump

Does show up when:

  1. open the plunker (the url from previewing in windows mode) in a new tab
  2. press refresh
  3. open DevTools and take a heap dump

So the refresh of the tab (even with closed DevTools) seems to trigger the leak.
@swar30 Can you confirm this on the plunker and also for your real application?

I used Chrome 32.0.1700.68 beta on Mac OSX.

@tbosch
Copy link
Contributor

tbosch commented Jan 2, 2014

Does NOT show up when:

  1. open the plunker (the url from previewing in windows mode) in a new tab
  2. press refresh
  3. click 'open', 'hide', 'open', 'hide'
  4. open DevTools and take a heap dump -> shows only 1 instance of ChildScope

@tbosch
Copy link
Contributor

tbosch commented Jan 2, 2014

Changed the plunker to change the view every 10ms and plotted the memory graph. It does not increase over time.
I did this after a refresh of the window, and after about 5 min I took a memory snapshot and had only 1 instance of ChildScope.

screen shot 2014-01-02 at 10 24 17 am

So in total, I can't reproduce this as a memory leak, but only as a late garbage collection by Chrome.

@tbosch
Copy link
Contributor

tbosch commented Jan 2, 2014

I am sorry, we need another test case to reproduce your problem...

@swar30
Copy link
Author

swar30 commented Jan 2, 2014

It looks like in the plunker when I do see the leaked scopes, if I trigger GC a few times (from timeline) and then take a heap dump then indeed the extra scopes go away (as you said, looks like late GC). But they stayed in our app, I will try to check this again and see if I can produce a better reproduction.

@tbosch
Copy link
Contributor

tbosch commented Jan 2, 2014

This would be great! By they way, do you have a memory increase over a longer period of time in your app that is caused by the reference to $scope and which goes away if you remove that reference?

@tbosch
Copy link
Contributor

tbosch commented Jan 2, 2014

Updated the V8 issue for the late GC problem: https://code.google.com/p/v8/issues/detail?id=2073

@swar30
Copy link
Author

swar30 commented Jan 2, 2014

I was sure that it was related to $scope references and that using this instead of scope inside functions on scope resolved it, but now i'm not 100% sure, it might have just made GC collect it faster, I need to verify again and it will take some time to do so on our app. But from what I can see in our case, even if I force GC to run several times we are still leaking scopes, it might have been that I mistaken the late GC as root cause for scope leak and there might be some other issue and late GC was just the first to manifest, need deeper investigation.
I think I should have more info by next week.

I do need to note that the GC problem thread mentioned above does look familiar when compared to our heap dump, many of the leaked object start thier retaining tree some where inside (system / Map).

@tbosch
Copy link
Contributor

tbosch commented Jan 2, 2014

Thanks, and looking forward to updated!

On Thu, Jan 2, 2014 at 11:37 AM, swar30 notifications@github.com wrote:

I was sure that it was related to $scope references and that using this
instead of scope inside functions on scope resolved it, but now i'm not
100% sure, it might have just made GC collect it faster, I need to verify
again and it will take some time to do so on our app. But from what I can
see in our case, even if I force GC to run several times we are still
leaking scopes, it might have been that I mistaken the late GC as root
cause for scope leak and there might be some other issue and late GC was
just the first to manifest, need deeper investigation.
I think I should have more info by next week.

I do need to note that the GC problem thread mentioned above does look
familiar when compared to our heap dump, many of the leaked object start
thier retaining tree some where inside (system / Map).


Reply to this email directly or view it on GitHubhttps://github.com//issues/5527#issuecomment-31478131
.

@BankRen
Copy link

BankRen commented Jan 6, 2014

@tbosch @swar30 , please check this comment #2624 (comment)
maybe has some help

@swar30
Copy link
Author

swar30 commented Jan 6, 2014

Will check

@swar30
Copy link
Author

swar30 commented Jan 6, 2014

We do have nested ng-repeat in our app, but I had the leak when I removed all the view (left only <div></div>) and remained only with controller code which added functions to scope.

I still need to re-run this scenario and see if it is as tbosch said above in this thread, related to Chrome GC issues.

@ghost ghost assigned tbosch Jan 6, 2014
@swar30
Copy link
Author

swar30 commented Jan 10, 2014

Checked again, looks like using "this" instead of $scope inside function does not solve the issue after all.
What does seem to solve the issue is to set $scope to null on $destroy of scope.
I have new reproduction of the leak at http://plnkr.co/edit/N4SLX2?p=preview.
There you can see that we are using ui-router and we have two routes, route 1 has in it's controller a function for which $scope of controller is in it's scope.
If we are to click on route 1, and then click on route 2, we expect the scope of route 1 to be released.

I checked this by forcing GC a few times from timeline in Chrome and than taking a heap dump, there I see that I have 3 scopes, one of which is the scope for route 1.

This does not look like a late GC, but rather like a GC bug which manifests itself when we have a function on $scope which has $scope in it's closure / scope meaning any function you put on $scope inside your controller.

@tbosch tbosch modified the milestones: 1.2.12, 1.2.11 Feb 3, 2014
@tbosch
Copy link
Contributor

tbosch commented Feb 6, 2014

I am closing this issue as it's not an Angular related issue. Please see the linked Chrome issue.

Please note that this issue does not mean that the garbage is increasing over time. There is garbage, but it stays constant while the user is using the app.

Thanks for contributing.

@tbosch tbosch closed this as completed Feb 6, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants