Permalink
Browse files

timers: handle negative or non-numeric timeout values

Follows browser behaviour by scheduling the callback on the next tick.

Fixes #593.
  • Loading branch information...
1 parent b66d225 commit 8ad75f9bbd41bd6703f5d26be96f78eac9b556b4 @bnoordhuis committed Jul 11, 2011
Showing with 120 additions and 51 deletions.
  1. +18 −19 lib/timers_legacy.js
  2. +31 −32 lib/timers_uv.js
  3. +71 −0 test/simple/test-timers.js
View
@@ -23,6 +23,9 @@ var Timer = process.binding('timer').Timer;
var L = require('_linklist');
var assert = require('assert').ok;
+// Timeout values > TIMEOUT_MAX are set to 1.
+var TIMEOUT_MAX = 2147483647; // 2^31-1
+
var debug;
if (process.env.NODE_DEBUG && /timer/.test(process.env.NODE_DEBUG)) {
debug = function() { require('util').error.apply(this, arguments); };
@@ -148,16 +151,15 @@ exports.active = function(item) {
exports.setTimeout = function(callback, after) {
var timer;
- if (after <= 0) {
- // Use the slow case for after == 0
- timer = new Timer();
- timer.callback = callback;
- } else {
- timer = { _idleTimeout: after, _onTimeout: callback };
- timer._idlePrev = timer;
- timer._idleNext = timer;
+ after = ~~after;
+ if (after < 1 || after > TIMEOUT_MAX) {
+ after = 1; // schedule on next tick, follows browser behaviour
}
+ timer = { _idleTimeout: after, _onTimeout: callback };
+ timer._idlePrev = timer;
+ timer._idleNext = timer;
+
/*
* Sometimes setTimeout is called with arguments, EG
*
@@ -173,18 +175,10 @@ exports.setTimeout = function(callback, after) {
callback.apply(timer, args);
};
- if (timer instanceof Timer) {
- timer.callback = c;
- } else {
- timer._onTimeout = c;
- }
+ timer._onTimeout = c;
}
- if (timer instanceof Timer) {
- timer.start(0, 0);
- } else {
- exports.active(timer);
- }
+ exports.active(timer);
return timer;
};
@@ -202,6 +196,11 @@ exports.clearTimeout = function(timer) {
exports.setInterval = function(callback, repeat) {
var timer = new Timer();
+ repeat = ~~repeat;
+ if (repeat < 1 || repeat > TIMEOUT_MAX) {
+ repeat = 1; // schedule on next tick, follows browser behaviour
+ }
+
if (arguments.length > 2) {
var args = Array.prototype.slice.call(arguments, 2);
timer.callback = function() {
@@ -211,7 +210,7 @@ exports.setInterval = function(callback, repeat) {
timer.callback = callback;
}
- timer.start(repeat, repeat ? repeat : 1);
+ timer.start(repeat, repeat);
return timer;
};
View
@@ -23,6 +23,9 @@ var Timer = process.binding('timer_wrap').Timer;
var L = require('_linklist');
var assert = require('assert').ok;
+// Timeout values > TIMEOUT_MAX are set to 1.
+var TIMEOUT_MAX = 2147483647; // 2^31-1
+
var debug;
if (process.env.NODE_DEBUG && /timer/.test(process.env.NODE_DEBUG)) {
debug = function() { require('util').error.apply(this, arguments); };
@@ -146,44 +149,35 @@ exports.active = function(item) {
exports.setTimeout = function(callback, after) {
var timer, c, args;
- if (after <= 0) {
- // Use the slow case for after == 0
- timer = new Timer();
- timer.ontimeout = callback;
+ after = ~~after;
+ if (after < 1 || after > TIMEOUT_MAX) {
+ after = 1; // schedule on next tick, follows browser behaviour
+ }
+
+ timer = { _idleTimeout: after };
+ timer._idlePrev = timer;
+ timer._idleNext = timer;
+ if (arguments.length <= 2) {
+ timer._onTimeout = callback;
+ } else {
+ /*
+ * Sometimes setTimeout is called with arguments, EG
+ *
+ * setTimeout(callback, 2000, "hello", "world")
+ *
+ * If that's the case we need to call the callback with
+ * those args. The overhead of an extra closure is not
+ * desired in the normal case.
+ */
args = Array.prototype.slice.call(arguments, 2);
timer._onTimeout = function() {
callback.apply(timer, args);
- timer.close();
- }
-
- timer.start(0, 0);
- } else {
- timer = { _idleTimeout: after };
- timer._idlePrev = timer;
- timer._idleNext = timer;
-
- if (arguments.length <= 2) {
- timer._onTimeout = callback;
- } else {
- /*
- * Sometimes setTimeout is called with arguments, EG
- *
- * setTimeout(callback, 2000, "hello", "world")
- *
- * If that's the case we need to call the callback with
- * those args. The overhead of an extra closure is not
- * desired in the normal case.
- */
- args = Array.prototype.slice.call(arguments, 2);
- timer._onTimeout = function() {
- callback.apply(timer, args);
- }
}
-
- exports.active(timer);
}
+ exports.active(timer);
+
return timer;
};
@@ -203,12 +197,17 @@ exports.clearTimeout = function(timer) {
exports.setInterval = function(callback, repeat) {
var timer = new Timer();
+ repeat = ~~repeat;
+ if (repeat < 1 || repeat > TIMEOUT_MAX) {
+ repeat = 1; // schedule on next tick, follows browser behaviour
+ }
+
var args = Array.prototype.slice.call(arguments, 2);
timer.ontimeout = function() {
callback.apply(timer, args);
}
- timer.start(repeat, repeat ? repeat : 1);
+ timer.start(repeat, repeat);
return timer;
};
View
@@ -0,0 +1,71 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var common = require('../common');
+var assert = require('assert');
+
+var inputs = [
+ undefined,
+ null,
+ true,
+ false,
+ '',
+ [],
+ {},
+ NaN,
+ +Infinity,
+ -Infinity,
+ (1.0 / 0.0), // sanity check
+ parseFloat('x'), // NaN
+ -10,
+ -1,
+ -0.5,
+ -0.0,
+ 0,
+ 0.0,
+ 0.5,
+ 1,
+ 1.0,
+ 10,
+ 2147483648 // browser behaviour: timeouts > 2^31-1 run on next tick
+];
+
+var timeouts = [];
+var intervals = [];
+
+inputs.forEach(function(value, index) {
+ setTimeout(function() {
+ timeouts[index] = true;
+ }, value);
+
+ var handle = setInterval(function() {
+ clearInterval(handle); // disarm timer or we'll never finish
+ intervals[index] = true;
+ }, value);
+});
+
+process.on('exit', function() {
+ // assert that all timers have run
+ inputs.forEach(function(value, index) {
+ assert.equal(true, timeouts[index]);
+ assert.equal(true, intervals[index]);
+ });
+});

0 comments on commit 8ad75f9

Please sign in to comment.