Can not stop the function running on setTimeout

Question

I have the following javascript function that runs every 10 seconds to sync some data with the database.

function sync(){
  latestID = $("#dataDiv tbody tr:first").attr("data-key");
  var url = '<?=$model['fn']?>';
  $.ajax({
    url: url,
    type: 'post',
    dataType: 'json',
    data:{latestID: latestID},
    success: function (result) {
      //Do some stuff
    }
  });
  setTimeout(sync, 10000);
}

This function starts to execute when a Activate Sync checkbox is checked. but once it is running, it does not stop when unckecking the checkbox. Below is what I already have tried that does do anything and the function gets executing until the page is reloaded (the checkbox is unchecked by default)

function checkSync(){
  var doSync;

  if($('#keepChecking').is(':checked')){
    doSync = sync();
    //alert("checked"); return false;
  }else{
    clearTimeout(doSync);
    return false;
    //alert("not checked"); return false;
  }
}

add a return: `return setTimeout(sync, 10000);` - but that will only work on the first invocation. so change the setTimeout to checkSync: `return setTimeout(checkSync, 10000);` - probably needs other changes as well, but might get you closer. — freedomn-m, Oct 23 '17 at 12:22
returning the timeout handle would not work as expected since it would create a new timeout each time the timeout runs out — GottZ, Oct 23 '17 at 12:24
@freedomn-m I already tried that, it keeps running with the return too. — M Reza Saberi, Oct 23 '17 at 12:24
Sorry, extended my comment after the initial, most obvious issue. It's not as simple as just returning or re-checking as you will have two methods to trigger and you may end up running it twice or multiple times. Next step would be to always call clearTimeout() and only start if checked, then it can be called multiple times from multiple triggers. — freedomn-m, Oct 23 '17 at 12:25
The easiest concept would be to add a variable outside the methods (not necessarily global) that keeps track of the `setTimeout` value - then on 2nd/3rd etc calls it will update. I also notice your setTimeout is *outside* the ajax call, so you could remove the setInterval from inside the function and add `setInterval(sync, 10000)` to the call instead — freedomn-m, Oct 23 '17 at 12:29
try adding `var doSync ` out of the scope of function. i-e outside `chksync()` — Muhammad Usman, Oct 23 '17 at 12:30
@Akxe as long as it's not called every time inside sync() (ie not a replacement for the current setInterval), that would also work, and likely be cleaner. — freedomn-m, Oct 23 '17 at 12:30
@freedomn-m so it does go to the first function (checkSync) to check if the checkbox is checked. and then running the sync function. Is it an appropriate approach? — M Reza Saberi, Oct 23 '17 at 12:37
What is shown does not run every 10 seconds unless you have another interval timer calling checkSync(). Something missing here. setTimeout only executes once — charlietfl, Oct 23 '17 at 12:37
Ok, I think I know what is going on... apart from having to solve the clearing of the timeouts as pointed out in multiple answers below, you are not cancelling the last ajax request, so even after you uncheck it, the last one call will get a response, and the code inside "//Do some stuff" will be executed too https://stackoverflow.com/questions/446594/abort-ajax-requests-using-jquery — , Oct 23 '17 at 13:00
@charlietfl there is no other interval timer calling checkSync(). The proble was to inform the setTimeout to stop (to call the clearTimeout) which seems like there was a need to run checkSync() regularly so that we could decide if it is needed to call clearTimeout. — M Reza Saberi, Oct 23 '17 at 13:07
@RomanCortes What I actually have inside that `//Do some stuff` is creating a table row and prepend the new rows to the records table! — M Reza Saberi, Oct 23 '17 at 13:13

score 0 · Answer 1 · answered Oct 23 '17 at 13:05

This should take care of the timeout and of the ajax request:

var ajaxRequest,
    syncTimeout;

function sync(){
  latestID = $("#dataDiv tbody tr:first").attr("data-key");
  var url = '<?=$model['fn']?>';
  ajaxRequest = $.ajax({
    url: url,
    type: 'post',
    dataType: 'json',
    data:{latestID: latestID},
    success: function (result) {
      //Do some stuff
    }
  });
  syncTimeout = setTimeout(sync, 10000);
}

function checkSync(){
  if($('#keepChecking').is(':checked')){
    sync();
    //alert("checked"); return false;
  }else{
    clearTimeout(syncTimeout);
    ajaxRequest.abort();
    return false;
    //alert("not checked"); return false;
  }
}

score -1 · Answer 2 · answered Oct 23 '17 at 12:31

You didn't attach doSync to the setTimeOut function and declared it within the function, so not globally.

Do the following:

Declare var doSync; outside your functions to set it globally.
Edit your last line of the function to this: return setTimeout(sync, 10000);

Now doSync will contain the ID value returned by setTimeOut.

Example of a working code:

// Declare the variable globally
var mySync;

function setFunction() {
    // Set mySync to the ID returned by setTimeout (to use clearTimeout)
    mySync = setTimeout(function(){}, 3000);
}

function clearFunction() {
    // Clear timeout with the ID
    clearTimeout(mySync);
}

Did you even try it out and actually understand what I explained in my answer? — Huso, Oct 23 '17 at 12:43

score -1 · Answer 3 · answered Oct 23 '17 at 12:34

you need to use some kind of terminator since you are constantly running timeouts.

you could do something like this:

// let's grab the html elements
const counter = document.getElementById('counter');
const start = document.getElementById('start');
const stop = document.getElementById('stop');

// define a counter
let count = 0;

// function that runs the payload
const runTimer = terminator => {
  counter.textContent = ++count;
  // on each run assign the new timeout to the terminator object
  terminator.timeout = setTimeout(()=> runTimer(terminator), 100);
};

// define an object we use to access the most recent timeout
const terminator = {};
// when i click start → run the payload
start.addEventListener('click', e => runTimer(terminator));
// when i click stop → terminate the last created timeout reference.
stop.addEventListener('click', e => clearTimeout(terminator.timeout));

<div id="counter">0</div>
<br>
<button id="start">start</button>
<button id="stop">stop</button>

well.. you don't need to use an object for this but maybe this gives you an idea of how you could create some kind of data binding.

M Reza Saberi · Accepted Answer · 2017-10-23T13:33:59.290

-1

Just as freedomn-m said in the comments of the question, It works fine if I call checkSync function in the setTimeout().

setTimeout(checkSync, <?=$model['aud'];?>);

The proble was to inform the setTimeout to stop (to call the clearTimeout) which seems like there was a need to run checkSync() regularly so that we could decide if it is needed to call clearTimeout. It now checks if the checkbox state is changed every 10 seconds to decide wether do the sync() or not.

Thank you all for putting your time on this.

edited Oct 23 '17 at 13:33

answered Oct 23 '17 at 12:51

M Reza Saberi

7,134
9
47
76

So, you were not listening to changes in the checkbox element? Wow. – Oct 23 '17 at 14:07

Can not stop the function running on setTimeout

4 Answers4