views:

555

answers:

6

I'm working on a proxy server checker and have the following code to start the requests at intervals of roughly 5 seconds using the setTimeout function;

        function check() {

            var url = document.getElementById('url').value;
            var proxys = document.getElementById('proxys').value.replace(/\n/g,',');

            var proxys = proxys.split(",");

            for (proxy in proxys) {

                var proxytimeout = proxy*5000;

                t = setTimeout(doRequest, proxytimeout, url, proxys[proxy]);

            }
        }

However I can't stop them once their started!

        function stopcheck() {

            clearTimeout(t);

        }

A fix or better method will be more that appreciated.

Thank you Stack Overflow Community!

+2  A: 

Define t outside of both functions first. Additionally, you're overwriting t with each iteration your for loop. Perhaps building a collection of references, and then to stop them you cycle through and clearTimeout on each.

Jonathan Sampson
+3  A: 

Where is 't' being defined? It keeps being redefined in the for loop, so you will loose track of each timeout handle...

You could keep an array of handles:

var aTimeoutHandles = new Array();
var iCount = 0;
for (proxy in proxys) {

    var proxytimeout = proxy*5000;

    aTimeoutHandles[iCount++] = setTimeout(doRequest, proxytimeout, url, proxys[proxy]);

}
Robert
If you're setting multiple timeouts you'll need to keep track of each one, the way you're currently doing it every handle for a timeout is getting nuked when every new timer is created. Also be sure to define your handles array in a scope where both functions can see.
Cryo
Oh - can you not create multiple timeouts like that? I'm sure I've done that before somewhere!Re: array - Sure - I was just demonstrating that you could store the timeout handles in an array. I guess I should have mentioned that the scope of the variable was important...
Robert
@Cryo: Your comment is correct for the OP's question, but Robert's answer stores the handles in an array.
T.J. Crowder
A: 

You overwrite t each time you set the interval. Thus you only end up clearing the last one set.

Anon.
A: 

You have a few problems there:

  1. The main one is that you're overwriting t on each iteration of your for loop; you need an array of ts for your structure to work.
  2. You're using for..in to loop through the indexes of the array. That's not what for..in is for (although there are a lot of people confused about that; see this article). for..in loops through the property names of an object, not the indexes of an array, and therefore this usage breaks in non-trivial situations. Just use an old-fashioned for loop.
  3. You're declaring proxys twice. This is actually harmless, but...
  4. You're not declaring proxy at all (which isn't harmless; it becomes an implicit global).

I've updated the code in Jordan's excellent answer to address those.

T.J. Crowder
A: 

Looks like you're setting multiple timeouts (one for each proxy), but trying to save them in the same variable. You probably need to use an array there, instead of a simple variable.

Mark Bessey
+4  A: 

There are 2 major problems with your code:

  1. t is overwritten for each timeout, losing the reference to the previous timeout each iteration.
  2. t is may not be a global variable, thus stopcheck() might not be able to "see" t.

Updated functions:

function check() {
    var url         = document.getElementById('url').value;
    var proxys      = document.getElementById('proxys').value.replace(/\n/g,',');
    var timeouts    = [];
    var index;
    var proxytimeout;

    proxys = proxys.split(",");
    for (index = 0; index < proxys.length; ++index) {
        proxytimeout                = index * 5000;
        timeouts[timeouts.length]   = setTimeout(
            doRequest, proxytimeout, url, proxys[index];
        );
    }

    return timeouts;
}

function stopcheck(timeouts) {
    for (var i = 0; i < timeouts.length; i++) {        
        clearTimeout(timeouts[i]);
    }
}

Example of use:

var timeouts = check();

// do some other stuff...

stopcheck(timeouts);
Jordan Ryan Moore
`t` *was* a global variable, as far as we could tell from his posted code. it wasn't declared in either function, meaning either it was declared globally, or it wasn't. In the latter case, it would become an implicit property of `window` (e.g., an implicit global, horrible though those are). Your approach is, of course, better than that.
T.J. Crowder
Took the liberty of fixing the other problems in the code.
T.J. Crowder