angular / angular.js

AngularJS - HTML enhanced for web apps!
https://angularjs.org
MIT License
58.82k stars 27.5k forks source link

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

Closed swar30 closed 10 years ago

swar30 commented 10 years ago

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 https://github.com/angular/angular.js/issues/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

tbosch commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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

swar30 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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

swar30 commented 10 years ago

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 commented 10 years ago

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/angular/angular.js/issues/5527#issuecomment-31478131 .

BankRen commented 10 years ago

@tbosch @swar30 , please check this comment https://github.com/angular/angular.js/issues/2624#issuecomment-31454695 maybe has some help

swar30 commented 10 years ago

Will check

swar30 commented 10 years ago

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.

swar30 commented 10 years ago

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 commented 10 years ago

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.