Fix timers

lib/timers.js

@@ -56,12 +56,12 @@ function listOnTimeout() {
 
   debug('timeout callback %d', msecs);
 
-  var now = Timer.now();
-  debug('now: %s', now);
-
-  var diff, first, threw;
+  var now, diff, first, threw;
   while (first = L.peek(list)) {
+    now = Timer.now();
+    debug('now: %s', now);
     diff = now - first._idleStart;
+
     if (diff < msecs) {
       list.start(msecs - diff, 0);
       debug('%d list wait because diff is %d', msecs, diff);
@@ -198,7 +198,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);
     };
   }
 
@@ -240,15 +240,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;
+      this._idleTimeout = repeat;
       exports.active(timer);
+      callback.apply(this, args);
     }
   }
 };
@@ -281,7 +281,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

@@ -35,6 +35,7 @@ class TimerWrap : public HandleWrap {
     constructor->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "kOnTimeout"),
                      Integer::New(env->isolate(), kOnTimeout));
 
+    env->SetTemplateMethod(constructor, "updateTime", UpdateTime);
     env->SetTemplateMethod(constructor, "now", Now);
 
     env->SetProtoMethod(constructor, "close", HandleWrap::Close);
@@ -119,6 +120,12 @@ class TimerWrap : public HandleWrap {
     wrap->MakeCallback(kOnTimeout, 0, nullptr);
   }
 
+  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) {
     Environment* env = Environment::GetCurrent(args);
     uv_update_time(env->event_loop());

test/parallel/test-timers-busy.js

@@ -0,0 +1,27 @@
+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/parallel/test-timers-intervals.js

@@ -0,0 +1,50 @@
+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/parallel/test-timers-this.js

@@ -1,7 +1,8 @@
 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;
@@ -11,6 +12,7 @@ var immediateArgsHandler = setImmediate(function () {
   immediateArgsThis = this;
 }, "args ...");
 
+
 var intervalHandler = setInterval(function () {
   clearInterval(intervalHandler);
 
@@ -23,6 +25,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;
 });
@@ -31,13 +48,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);
 });

test/parallel/test-timers-unref-call.js

@@ -1,8 +1,6 @@
 var common = require('../common');
 
 var Timer = process.binding('timer_wrap').Timer;
-Timer.now = function() { return ++Timer.now.ticks; };
-Timer.now.ticks = 0;
 
 var t = setInterval(function() {}, 1);
 var o = { _idleStart: 0, _idleTimeout: 1 };