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

Using $timeout causes a memory leak #1522

Closed
jaitaiwan opened this issue Nov 3, 2012 · 28 comments
Closed

Using $timeout causes a memory leak #1522

jaitaiwan opened this issue Nov 3, 2012 · 28 comments

Comments

@jaitaiwan
Copy link

After much fun learning about memory leaks and how to use chrome's profiler and timeline. I finally found out the cause of my memory leak.

It appears that natively, a memory leak is created when passing a string to setTimeout instead of a function reference: http://pavel.kuzub.com/settimeout-memory-leak

An example of the code I was using that was faulty:

doTime = ->
    now = new Date()
    hour = if now.getHours().toString().length == 1 then "0" + now.getHours() else now.getHours()
    minute = if now.getMinutes().toString().length == 1 then "0" + now.getMinutes() else now.getMinutes()
    second = if now.getSeconds().toString().length == 1 then "0" + now.getSeconds() else now.getSeconds()
    $scope.clock = "#{hour}:#{minute}:#{second}"
    $timeout doTime, 2000
doTime()

changing

$timeout doTime, 2000

to

setTimeout doTime, 2000

Fixed the issue.

I assume that there is probably a call to $scope.$eval somewhere in $timeout which is what is causing the issue.

@IgorMinar
Copy link
Contributor

we don't call settimeout with strings anywhere, so something else must be going on.

how did you diagnose the code above as leaky? do you see retained objects or something that could pinpoint the cause?

@jaitaiwan
Copy link
Author

I kept receiving a chrome aww snap. After finding out what an aww snap
could be I was introduced to chromes profile and timeline tabs in the dev
console.

It showed the heap size growing as time passed and garbage collection
wasn't able to remove all instances. I made sure there were no connections
to objects etc and finally found that article. Once I changed it to
setTimeout garbage collection was able to happen correctly.

Sorry for the lack of technical terms, I've only delved into this the last
week.
On 06/11/2012 1:13 PM, "Igor Minar" notifications@github.com wrote:

we don't call settimeout with strings anywhere, so something else must be
going on.

how did you diagnose the code above as leaky? do you see retained objects
or something that could pinpoint the cause?


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

@BinaryMuse
Copy link

I happened upon this GitHub issue while diagnosing my own, similar problem. The code I was using was:

tick = ->
  $scope.time = ServerTime() // returns a promise based on some calculations
  $timeout tick, 5000
tick()

I noticed that after a while, I would get the "Aww snap" Chrome message, and after a while I noticed I could make it happen much faster by setting a a lower timeout; using a value of 10 caused my tab to become unresponsive and crash in under a minute. Heap snapshots show associated memory growth.

After some fiddling, I changed the code to:

tick = ->
  $scope.time = ServerTime() // returns a promise based on some calculations
  $scope.$apply()
  setTimeout tick, 5000
tick()

And I've found it works much better (even with a low timeout).

I hope this helps; please let me know if I can provide additional information.

@petebacondarwin
Copy link
Member

Could you provide a working demo of this problem? This simple
implementation of what you describe doesn't have this problem:
http://plnkr.co/edit/wZyC4w.
I wonder if there is something inside your ServerTime function that is
actually the problem here?
Pete

On 12 November 2012 00:24, Brandon Tilley notifications@github.com wrote:

I happened upon this GitHub issue while diagnosing my own, similar
problem. The code I was using was:

tick = ->
$scope.time = ServerTime() // returns a promise based on some calculations
$timeout tick, 5000tick()

I noticed that after a while, I would get the "Aww snap" Chrome message,
and after a while I noticed I could make it happen much faster by setting a
a lower timeout; using a value of 10 caused my tab to become unresponsive
and crash in under a minute. Heap snapshots show associated memory growth.

After some fiddling, I changed the code to:

tick = ->
$scope.time = ServerTime() // returns a promise based on some calculations
$scope.$apply()
setTimeout tick, 5000tick()

And I've found it works much better (even with a low timeout).

I hope this helps; please let me know if I can provide additional
information.


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

@IgorMinar
Copy link
Contributor

I wonder if this is related to #1313

@IgorMinar
Copy link
Contributor

I can't repro this either. Anybody can provide more info? Ideally a working fiddle/plunker/html that leaks.

@BinaryMuse
Copy link

I've done some digging in the Chrome inspector, and it does look like my leak is related to the promises I use in ServerTime. Interestingly enough, I don't see the same issue with setTimeout. If I can get it down to a minimal example, I'll post a link and share some heap snapshots.

@jaitaiwan
Copy link
Author

I was using angular 1.0.1 and initially it looked like this http://plnkr.co/edit/upkYuG was a replication of the problem but it doesn't seem to increase beyond a certain size. You could potentially take a look at my github to see where it was there jaitaiwan/CPresent pub-src/assets/js/compiled/presenter.coffee

TL;DR: Same code in my app as in plunkr http://plnkr.co/edit/upkYuG and can't replicate.

@IgorMinar
Copy link
Contributor

@BinaryMuse any luck? example code would be awesome. if you can't repro it outside of your app having a heap snapshot would help too, though that's far from ideal, but definitely better than nothing. cc: @jaitaiwan

@jaitaiwan
Copy link
Author

Unfortunately, I don't have a specific commit I can checkout to reproduce the issue, without re-factoring the commit to remove the other bugs which also prevented proper garbage collection. Unless of course you don't mind having a heap with that stuff in there as well?

@IgorMinar
Copy link
Contributor

@jaitaiwan Can't you just swap setTimeout for $timeout and see if you can still reproduce the problem?

@jaitaiwan
Copy link
Author

Here is a heap snapshot. https://docs.google.com/open?id=0B3xElvhp0JIhUW9jbUxzQkF0Ujg

@karlgoldstein
Copy link

This example here causes a rapid memory leak in both Chrome and FF (OS X) for me:

http://plnkr.co/edit/b25pmGVQyZJatuCwIRWU

I could only reproduce the leak when running the timeout inside a service, rather than in a controller.

Replacing $timeout with window.setTimeout stops the leak.

@Cufeadir
Copy link

I was able to reproduce the issue with the example that @petebacondarwin posted at http://plnkr.co/edit/wZyC4w by changing app.js line 11 to: return $timeout(tick, 1)

This is what the code produced by the CoffeeScript @BinaryMuse posted does. In CoffeeScript, adding a line containing just "null" to the end of the method (to make it return null) avoids the problem.

@BinaryMuse
Copy link

@Cufeadir Aha! Thank you -- I should have realized this was the problem, as $timeout returns a promise that resolves to the return value of the fn argument passed to it. Since CoffeeScript implicitly returns the value of the last expression in a function à la Ruby, the call to $timeout returned a promise that resolved to the promise that gets returned from the inner call to $timeout, etc.

@IgorMinar As mentioned by @Cufeadir, simply adding null as the return value at the end of my fn argument takes care of the issue for me, and my guess is it fixes the issue for anyone else who calls $timeout at the end of their own fn.

@godmar
Copy link
Contributor

godmar commented Apr 5, 2013

What an insidious problem. We just encountered the same issue. It's very natural to write re-arming timers in CoffeeScript this way, and the interaction of CoffeeScript's default returns and Angular's $timeout returning a promise is absolutely non-obvious. We tried to figure it out from heapdumps, which show a rapidly growing 'pending' array for the 'defer' object associated with the $timeout, but didn't succeed until we found this post.

It's not only a memory leak, but will eat up CPU time because there are (n-1) calls to defer.promise.then at the n-th invocation of the timeout.

@jimrhoskins
Copy link

I've encountered the same issue. Using $timeout to set up a polling interval was causing the memory usage to climb and eventually "aww snap".

Replacing

$timeout(recur, 1000)

with

setTimeout ->
  $scope.$apply -> recur()
, 1000

seems to fix the issue.

@x0nix
Copy link

x0nix commented Aug 9, 2013

Just finished my memory leak hunt and finally found this issue. Based on some experiments the problem seems to be solved by NOT returning the recursive call (which cofeescript does by default).

this seems to work ok:

tick = ->
    $timeout tick, 5000
    return
tick()

this results in memory leak:

tick = ->
    $timeout tick, 5000  # same as return $timeout tick, 5000
tick()

@godmar
Copy link
Contributor

godmar commented Aug 9, 2013

@xonix: that's what @Cufeadir said and @BinaryMuse confirmed.

You can write either 'return', 'null', or 'return null' - they'll all work so that the coffeescript compiler will produce code that will not return the return value of $timeout, which is what's causing the leak.

@petebacondarwin
Copy link
Member

Can someone provide a PR doc update to $timeout to make it clear how to avoid this in both JS and CS?
Meanwhile we should close this.

@ljmagiera
Copy link

I really hate to bring up such an old issue, and I apologize in advance if this is the wrong way to ask my question. But this page is one of the closest things I can find for my problems. As a matter of fact, the plunkr of petebacondarwin ([(http://plnkr.co/edit/wZyC4w)]) from way back on Nov 12, 2012 shows it exactly. I updated that plunkr to angular 1.5.5. I'm on Windows 7, Chrome Version 50.0.2661.94 m and this is what my dev tools shows when i run that page:
heap

Here's my plunkr [https://plnkr.co/edit/NhGdi3Q6djt3iMPHIhd1] and here's my chrome dev tools timeline :
heap2

This is a problem for me as I have a requirement to continuously poll a server and display the results on the page.

Does anyone on this thread have a suggestion on how I can improve my memory usage and allow GC to do what it's payed to do? Or is this the best I can hope for with angular? Would dropping back to raw javascript be a better option with my requirement?

Thank you so much for any help/ideas/feedback.

@gkalpak
Copy link
Member

gkalpak commented May 17, 2016

I can't reproduce the issue on Chrome v50.0.2661.102 - Windows 10 - Angular v1.5.5.
Have you manually invoked GC ?

@rajdeol
Copy link

rajdeol commented Dec 16, 2016

@ljmagiera Have you managed to solve your problem, I am having the same issue. I have a $http promise inside a $timeout and I am getting memory leak. My app is a single page and single controller app so $timeout.cancel on $scope.$destroy is not much of a help

@gkalpak
Copy link
Member

gkalpak commented Dec 16, 2016

@rajdeol, if you can provide a reproduction of the leak, we can investigate and (if it turns out to be an Angular issue) fix it. Just saying... 😃

@rajdeol
Copy link

rajdeol commented Dec 22, 2016

@gkalpak I have created the plunker http://plnkr.co/edit/2HprjNPUU9IrBQxdw1Z0?p=preview

What I have noticed further is:

  1. Memory keeps on piling up with each call
  2. If I do a force garbage collection in chrome dev-tools it come back to normal
  3. I suspect it might be due to the promise so, I tried rejecting the promise in .finally()

Please suggest if anything can be done to reduce the memory piling up, it freezes the client machine before the garbage collection kicks in.

@godmar
Copy link
Contributor

godmar commented Dec 22, 2016

@rajdeol what you see is normal and not indicative of a memory leak.

A memory leak occurs if objects are kept alive in a way that prevents garbage collection from freeing them. In this case, you would not see a decrease in used memory after a garbage collection occurs.

@rajdeol
Copy link

rajdeol commented Dec 22, 2016

@godmar Thanks for the clarification, but my issue is memory is keep on piling and garbage collection does not fire. Try and profile the plunker I have shared in the chrome dev-tools. Timeline will show you the memory build up.

@gkalpak
Copy link
Member

gkalpak commented Dec 22, 2016

@rajdeol: As @godmar said, it is normal (and expected) that memory builds up during an application's lifetime (and reduced periodically with garbage collection). I doubt that just building up GC-able memory freezes the client's browser (and if it does, then it sounds like a browser issue).

I don't think there is anything to do in Angular. If you feel Angular timeouts or promises could be more efficient in terms of the memory pressure they create, please post a minimal demo (without non-Angular dependencies) and the steps needed to reproduce the unnecessarily high memory built-up.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests