2

Does this code create any memory leaks? Or is there anything wrong with the code?

HTML:

<div id='info'></div>

Javascript:

var count = 0;
function KeepAlive()
{
 count++;
 $('#info').html(count);
 var t=setTimeout(KeepAlive,1000);
}
KeepAlive();

Run a test here: http://jsfiddle.net/RjGav/

Michel Gokan Khan
2,6734 gold badges32 silver badges57 bronze badges
asked May 27, 2011 at 17:53
1
  • I'm not certain if using setTimeout in the manner that you describe causes memory leaks. There are things you can try if you want to test for memory leaks on your own -- but if you choose to experiment, remember that different browsers leak for different reasons and in different ways. Commented May 27, 2011 at 18:05

4 Answers 4

5

You should probably use setInterval instead:

var count = 0;
function KeepAlive() {
 $('#info').html(++count);
}
var KAinterval = setInterval(KeepAlive, 1000);

You can cancel it if you ever need to by calling clearInterval(KAinterval);.

answered May 27, 2011 at 17:56
Sign up to request clarification or add additional context in comments.

2 Comments

Thanks g.d.d.c +1... is there any reason you're not using var in front of KAinterval? Just wondering.
@capdragon - A typo. Good catch.
5

I think this will leak because the successive references are never released. That is, the first call immediately creates a closure by referencing the function from within itself. When it calls itself again, the new reference is from the instance created on the first iteration, so the first one could again never be released.

You could test this theory pretty easily by changing the interval to something very small and watch the memory in chrome...

(edit) theory tested with your fiddle, actually, I'm wrong it doesn't leak, at least in Chrome. But that's no guarantee some other browser (e.g. older IE) isn't as good at garbage collecting.

But whether or not it leaks, there's no reason not to use setInterval instead.

answered May 27, 2011 at 17:58

Comments

3

This should not create a leak, because the KeepAlive function will complete in a timely manner and thus release all variables in that function. Also, in your current code, there is no reason to set the t var as it is unused. If you want to use it to cancel your event, you should declare it in a higher scope.

Other than that, I see nothing "wrong" with your code, but it really depends on what you are trying to do. For example, if you are trying to use this as a precise timer, it will be slower than a regular clock. Thus, you should either consider either setting the date on page load and calculating the difference when you need it or using setInterval as g.d.d.c suggested.

answered May 27, 2011 at 18:07

Comments

2

It is good to have setInterval method like g.d.d.c mentioned.
Moreover, it is better to store $('#info') in a variable outside the function.

Checkout http://jsfiddle.net/RjGav/1/

answered May 27, 2011 at 18:05

3 Comments

Thanks, could you please elaborate on why it's better to store the variable outside the function... also the jsfiddle link you posted does not run.
$('#info') runs everytime this function runs which is not needed as the value returned from it would never change. By moving this call outside the function, this unnecessary computing is avoided.
@capdragon: Every time you run $("#info"), jQuery has to find the element(s) again. If you store $("#info") in a variable and reuse the variable later, then jQuery only has to find the element(s) a single time (when the variable is defined).

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.