jquery setTimeout too much recursion

Question

I have read from multiple places that setTimeout() is preferable to setInterval() when setting something up to basically run forever. The code below works fine but after about an hour of running Firefox (38.0.1) throws an error of too much recursion.

Essentially I have it grabbing a very small amount of text from counts.php and updating a table with that information. The whole call and return takes about 50ms according to the inspectors. I'm trying to have it do this every x seconds as directed by t.

I suspect if I switch to setInterval() this would probably work, but I wasn't sure what the current state of the setTimeout() vs setInterval() mindset is as everything I've been finding is about 3-5 years old.

$(document).ready(function() {
    t = 3000;
    $.ajaxSetup({cache: false});

     function countsTimer(t) {
        setTimeout(function () {
            $.getJSON("counts.php", function (r) {
                $(".count").each(function(i,v) {
                    if ($(this).html() != r[i]) {
                        $(this).fadeOut(function () {
                            $(this)
                                .css("color", ($(this).html() < r[i]) ? "green" : "red")
                                .html(r[i])
                                .fadeIn()
                                .animate({color: '#585858'}, 10000);
                        })
                    };
                });

                t = $(".selected").html().slice(0,-1) * ($(".selected").html().slice(-1) == "s" ? 1000 : 60000);

                countsTimer(t);
            });
        }, t);
    };
    countsTimer(t);
});

Update: This issue was resolved by adding the .stop(true, true) before the .fadeOut() animation. This issue only occurred in Firefox as testing in other browsers didn't cause any issues. I have marked the answer as correct in spite of it not being the solution in this particular case but rather it offers a good explanation in a more general sense.

I don't know why `setInterval` shouldn't be used. What were the reasons given? Sounds like spurious claims or reasons that are relevant only in specific situations. — , May 25 '15 at 18:50
Im wondering if clearing the timeout would fix this issue even im really not sure from where comes the recursion — A. Wolff, May 25 '15 at 18:51
@squint to avoid some issue if request makes more than delay to complete — A. Wolff, May 25 '15 at 18:52
Is the value of `t` what you expect it to be everytime you update it? Is the browser the active window the entire time, or does it lose focus? — , May 25 '15 at 18:56
@squint I know I've read it on at least 10 different sites that it's preferable to use setTimeout() but now that I actually want to find those pages again I can't. I believe it comes down to essentially what A. Wolff said. In this case, the php file is only returning a single json array of about 5 items and each only being 3 integers at most, all located on the same server. In this instance I believe that using setTimeout() recursively like this is probably way more than overkill, I prefer to code in a way that I don't have to really change anything in the future if I want to make it do more. — mbazdell, May 25 '15 at 18:58
@squint I've added a `console.log(t)` before to verify that it does update and have a value, but I've never considered that it would matter in this case if the browser lost focus. To answer your question, I'm not sure, and the browser looses focus all the time. — mbazdell, May 25 '15 at 19:18
I guess your issue is regarding too many animations being put in queue. I cant post any answer with workaround coz currently im on tablet, so im sorry but you could test it using .stop(true) — A. Wolff, May 25 '15 at 19:20
@A.Wolff for no good reason I was under the impression that animate() and fadeIn() would restart/overwrite if called upon an already running animation, rather than add to a queue. I have nothing to base it on, just an assumption as to how it would likely function. — mbazdell, May 25 '15 at 19:27
As I wouldn't want to end animations on elements that aren't updating, would adding `$(this).stop(true).fadeOut(function () {` work as I imagine it would only apply the animation to that element then? I'm off to read about the fx queue. — mbazdell, May 25 '15 at 19:31
Ya would work as you imagine it if i understand correctly your expected behaviour — A. Wolff, May 25 '15 at 19:36
Now maybe the issue comes from timeout being put in stack when FF window isn't focused as suggested by squint. I'm not sure how FF handles it. Have you got same issue on chrome e.g? — A. Wolff, May 26 '15 at 10:31

w3re · Accepted Answer · 2015-05-25T20:42:24.210

You should indeed switch to setInterval() in this case. The problem with setInterval() is that you either have to keep a reference if you ever want to clear the timeout and in case the operation (possibly) takes longer to perform than the timeout itself the operation could be running twice.

For example if you have a function running every 1s using setInterval, however the function itself takes 2s to complete due to a slow XHR request, that function will be running twice at the same time at some point. This is often undesirable. By using setTimout and calling that at the end of the original function the function never overlaps and the timeout you set is always the time between two function calls.

However, in your case you have a long-running application it seems, because your function runs every 3 seconds, the function call stack will increase by one every three seconds. This cannot be avoided unless you break this recursion loop. For example, you could only do the request when receiving a browser event like click on the document and checking for the time.

(function() 
{
    var lastCheck = Date.now(), alreadyRunning = false;
    document.addEventListener
    (
        "click", 
        function() 
        {
            if(!alreadyRunning && Date.now() - lastCheck > 3000) 
            {
                alreadyRunning = true;
                /* Do your request here! */
                //Code below should run after your request has finished
                lastCheck = Date.now();
                alreadyRunning = false;
            }
        }
    )
}());

This doesn't have the drawback setInterval does, because you always check if the code is already running, however the check only runs when receiving a browser event. (Which is normally not a problem.) And this method causes a lot more boilerplate.

So if you're sure the XHR request won't take longer than 3s to complete, just use setInterval().

Edit: Answer above is wrong in some aspects

As pointed out in the comments, setTimeout() does indeed not increase the call stack size, since it returns before the function in the timeout is called. Also the function in the question does not contain any specific recursion. I'll keep this answer because part of the question are about setTimeout() vs setInterval(). However, the problem causing the recursion error will probably be in some other piece of code since there is not function calling itself, directly or indirectly, anywhere in the sample code.

I should have added that the use of the page is that it's going to be displayed and never really interacted with. So it could sit there for hours without the mouse ever moving. Is the reason for not using setInterval only the chance that it may overlap and already running instance? As you used here, would adding a check for `alreadyRunning` pretty much alleviate that issue? — mbazdell, May 25 '15 at 19:39
In that case this is a textbook example of a correct use for setInterval(). — w3re, May 25 '15 at 19:40
Independent of the question if `setTimeout` or `setInverval` should be used, the answer is incorrect in the part about the function call stack increasing. `countsTimer` returns and removes itself from the stack before the function in `setTimeout` executes with the next `countsTimer` call. At least it should. — nomve, May 25 '15 at 20:24

jquery setTimeout too much recursion

1 Answers1