Fix timers

lib/timers.js

@@ -97,12 +97,12 @@ function listOnTimeout() {
 
   debug('timeout callback %d', msecs);
 
-  var now = Timer.now();
-  debug('now: %s', now);
-
-  var diff, first, hasQueue, threw;
+  var now, diff, first, hasQueue, threw;
   while (first = L.peek(list)) {
+    now = Timer.now();
+    debug('now: %d', now);
     diff = now - first._idleStart;
+
     if (diff < msecs) {
       list.start(msecs - diff, 0);
       debug('%d list wait because diff is %d', msecs, diff);
@@ -242,7 +242,7 @@ exports.setTimeout = function(callback, after) {
      */
     var args = Array.prototype.slice.call(arguments, 2);
     timer._onTimeout = function() {
-      callback.apply(timer, args);
+      callback.apply(this, args);
     }
   }
 
@@ -284,15 +284,15 @@ exports.setInterval = function(callback, repeat) {
   return timer;
 
   function wrapper() {
-    callback.apply(this, args);
-    // If callback called clearInterval().
-    if (timer._repeat === false) return;
     // If timer is unref'd (or was - it's permanently removed from the list.)
     if (this._handle) {
       this._handle.start(repeat, 0);
+      callback.apply(this, args);
+      Timer.updateTime();
     } else {
-      timer._idleTimeout = repeat;
-      exports.active(timer);
+      this._idleTimeout = repeat;
+      exports.active(this);
+      callback.apply(this, args);
     }
   }
 };
@@ -323,7 +323,7 @@ Timeout.prototype.unref = function() {
     if (delay < 0) delay = 0;
     exports.unenroll(this);
     this._handle = new Timer();
-    this._handle[kOnTimeout] = this._onTimeout;
+    this._handle[kOnTimeout] = this._onTimeout && this._onTimeout.bind(this);
     this._handle.start(delay, 0);
     this._handle.domain = this.domain;
     this._handle.unref();

src/timer_wrap.cc

@@ -57,6 +57,7 @@ class TimerWrap : public HandleWrap {
     constructor->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "kOnTimeout"),
                      Integer::New(env->isolate(), kOnTimeout));
 
+    NODE_SET_METHOD(constructor, "updateTime", UpdateTime);
     NODE_SET_METHOD(constructor, "now", Now);
 
     NODE_SET_PROTOTYPE_METHOD(constructor, "close", HandleWrap::Close);
@@ -152,6 +153,12 @@ class TimerWrap : public HandleWrap {
     wrap->MakeCallback(kOnTimeout, 0, NULL);
   }
 
+  static void UpdateTime(const FunctionCallbackInfo<Value>& args) {
+    Environment* env = Environment::GetCurrent(args.GetIsolate());
+    HandleScope scope(env->isolate());
+    uv_update_time(env->event_loop());
+  }
+
   static void Now(const FunctionCallbackInfo<Value>& args) {
     HandleScope handle_scope(args.GetIsolate());
     Environment* env = Environment::GetCurrent(args.GetIsolate());

test/simple/test-timers-busy.js

@@ -0,0 +1,48 @@
+// 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 DELAY = 50;
+var WINDOW = 5;
+
+function nearly(a, b) {
+  if (Math.abs(a-b) > WINDOW)
+    assert.strictEqual(a, b);
+}
+
+function work(ms) {
+  var start = Date.now();
+  while (Date.now() - start < ms);
+}
+
+function test() {
+  var start = Date.now();
+  setTimeout(function() {
+    work(DELAY + 20);
+    setTimeout(function() {
+      nearly(DELAY*3+20, Date.now() - start);
+    }, DELAY);
+  }, DELAY);
+}
+
+test();

test/simple/test-timers-intervals.js

@@ -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 ALLOWABLE_ERROR = 5; // [ms]
+
+function nearly(a, b) {
+  if (Math.abs(a-b) > ALLOWABLE_ERROR)
+    assert.strictEqual(a, b);
+}
+
+function test(period, busyTime, expectedDelta, isUnref, next) {
+  var res = [];
+
+  var prev = Date.now();
+  var interval = setInterval(function() {
+    var s = Date.now();
+    while (Date.now() - s < busyTime);
+    var e = Date.now();
+
+    res.push(s-prev);
+    if (res.length === 5) {
+      clearInterval(interval);
+      done();
+    }
+
+    prev = e;
+  }, period);
+
+  if (isUnref) {
+    interval.unref();
+    var blocker = setTimeout(function() {
+      assert(false, 'Interval works too long');
+    }, Math.max(busyTime, period) * 6);
+  }
+
+  function done() {
+    if (blocker) clearTimeout(blocker);
+    nearly(period, res.shift());
+    res.forEach(nearly.bind(null, expectedDelta));
+    process.nextTick(next);
+  }
+}
+
+test(50, 10, 40, false, function() { // ref, simple
+  test(50, 10, 40, true, function() { // unref, simple
+    test(50, 100, 0, false, function() { // ref, overlay
+      test(50, 100, 0, true, function() {}); // unref, overlay
+    });
+  });
+});

test/simple/test-timers-this.js

@@ -21,8 +21,9 @@
 
 var assert = require('assert');
 
-var immediateThis, intervalThis, timeoutThis,
-    immediateArgsThis, intervalArgsThis, timeoutArgsThis;
+var immediateThis, intervalThis, timeoutThis, intervalUnrefThis,
+    timeoutUnrefThis, immediateArgsThis, intervalArgsThis, timeoutArgsThis,
+    intervalUnrefArgsThis, timeoutUnrefArgsThis;
 
 var immediateHandler = setImmediate(function () {
   immediateThis = this;
@@ -32,6 +33,7 @@ var immediateArgsHandler = setImmediate(function () {
   immediateArgsThis = this;
 }, "args ...");
 
+
 var intervalHandler = setInterval(function () {
   clearInterval(intervalHandler);
 
@@ -44,6 +46,21 @@ var intervalArgsHandler = setInterval(function () {
   intervalArgsThis = this;
 }, 0, "args ...");
 
+var intervalUnrefHandler = setInterval(function () {
+  clearInterval(intervalUnrefHandler);
+
+  intervalUnrefThis = this;
+});
+intervalUnrefHandler.unref();
+
+var intervalUnrefArgsHandler = setInterval(function () {
+  clearInterval(intervalUnrefArgsHandler);
+
+  intervalUnrefArgsThis = this;
+}, 0, "args ...");
+intervalUnrefArgsHandler.unref();
+
+
 var timeoutHandler = setTimeout(function () {
   timeoutThis = this;
 });
@@ -52,13 +69,29 @@ var timeoutArgsHandler = setTimeout(function () {
   timeoutArgsThis = this;
 }, 0, "args ...");
 
+var timeoutUnrefHandler = setTimeout(function () {
+  timeoutUnrefThis = this;
+});
+timeoutUnrefHandler.unref();
+
+var timeoutUnrefArgsHandler = setTimeout(function () {
+  timeoutUnrefArgsThis = this;
+}, 0, "args ...");
+timeoutUnrefArgsHandler.unref();
+
+setTimeout(function() {}, 5);
+
 process.once('exit', function () {
   assert.strictEqual(immediateThis, immediateHandler);
   assert.strictEqual(immediateArgsThis, immediateArgsHandler);
 
   assert.strictEqual(intervalThis, intervalHandler);
   assert.strictEqual(intervalArgsThis, intervalArgsHandler);
+  assert.strictEqual(intervalUnrefThis, intervalUnrefHandler);
+  assert.strictEqual(intervalUnrefArgsThis, intervalUnrefArgsHandler);
 
   assert.strictEqual(timeoutThis, timeoutHandler);
   assert.strictEqual(timeoutArgsThis, timeoutArgsHandler);
+  assert.strictEqual(timeoutUnrefThis, timeoutUnrefHandler);
+  assert.strictEqual(timeoutUnrefArgsThis, timeoutUnrefArgsHandler);
 });