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

setTimeout(fn) does not work as expected #593

Closed
ry opened this issue Jan 28, 2011 · 18 comments
Closed

setTimeout(fn) does not work as expected #593

ry opened this issue Jan 28, 2011 · 18 comments

Comments

@ry
Copy link

ry commented Jan 28, 2011

never get timeout.

@bnoordhuis
Copy link
Member

Is this about test/pummel/test-timers.js sometimes failing when the system is under load?

@ry
Copy link
Author

ry commented Jul 11, 2011

i'm not sure what this was about. closing.

@ry ry closed this as completed Jul 11, 2011
@freewil
Copy link

freewil commented Jul 11, 2011

I think you meant that when you omit the second param, the timeout never actually occurs, instead of it happening immediately like one might think.

@7fe
Copy link

7fe commented Jul 11, 2011

Yup @ER88 is right the function is never called and a error is never produced.

@bnoordhuis
Copy link
Member

Right, that's a bug. Reopening.

@bnoordhuis bnoordhuis reopened this Jul 11, 2011
bnoordhuis added a commit to bnoordhuis/node that referenced this issue Jul 11, 2011
@bnoordhuis
Copy link
Member

@ry or @isaacs - can one of you review 73dcd4f?

@isaacs
Copy link

isaacs commented Jul 14, 2011

Looks mostly ok to me.

  1. What does it mean to supply a timeout less than 0? Should it maybe throw for anything negative?
  2. Number.POSITIVE_INFINITY is probably much higher than we can reasonably support. I think it should probably not allow any number greater than Math.pow(2,32) - 1 (the MAX_INT in javascript.)

@bnoordhuis
Copy link
Member

@isaacs

  1. It makes the callback run ASAP. Not that useful but it maintains backward compatibility.
  2. I'm not a fan of arbitrary limits. That check for infinities and NaNs is because they confuse libev no end.

@ry
Copy link
Author

ry commented Jul 14, 2011

we should match web browser semantics here. what is the web browser doing?

@freewil
Copy link

freewil commented Jul 14, 2011

I know in the latest version of Firefox, anything equal to or less than zero, the timeout just occurs immediately, same if you omit the second param all together

@ghost
Copy link

ghost commented Jul 14, 2011

@isaacs: What MAX_INT do you mean? Afaik, there is no MAX_INT nor Number.MAX_INT and maximal integer number is Math.pow(53)-1 (maximal UInt32 is another matter, of course).

EDIT: The maximal possible Date is enough a limit, I think. In specs, it's year 9999, I think; in v8, it is very near to the maximal integer I mentioned above (in numbers of milliseconds from epoch). If an ending Date may be constructed, ok, if not, problem,

@bnoordhuis
Copy link
Member

Firefox 3.6.18

  1. -1, -1000, NaN, +Infinity and -Infinity all make the callback run on the next tick.
  2. Values up to 2^31-1 schedule the callback in the future, 2^31 and up make it run on the next tick.
  3. Passing negative values to setInterval() in node makes it throw an exception so in that respect we're not compliant.

@bnoordhuis
Copy link
Member

@ER88 Not immediately, at least not in 3.6.18. It runs on the next tick, a couple of milliseconds later.

bnoordhuis added a commit to bnoordhuis/node that referenced this issue Sep 6, 2011
Follows browser behaviour by scheduling the callback on the next tick.

Fixes nodejs#593.
@bnoordhuis
Copy link
Member

Can someone review 8ad75f9? It's a standalone patch that implements the behaviour observed in FF 3.6.

@tshinnic
Copy link

Hello.... this is related to

#1877: Large timeouts for setTimeout stop propagation of event loops

please see comments there.

@isaacs
Copy link

isaacs commented Mar 15, 2012

Landed on 7fc835a. Thanks!

@tshinnic
Copy link

So... does this help/apply to

#1877: Large timeouts for setTimeout stop propagation of event loops

I see the limit to (2**31)-1 , so maybe?

@isaacs
Copy link

isaacs commented Mar 15, 2012

Yes, this ought to fix #1877 as well. I'll check it out to make sure.

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

No branches or pull requests

6 participants