From ad60db8e4efe78f09c016cae3946d68b3b2cd801 Mon Sep 17 00:00:00 2001
From: Paul Loyd <pavelko95@gmail.com>
Date: Tue, 13 Jan 2015 17:53:44 +0300
Subject: [PATCH 1/3] timers: fix `this` for unref'd timers
---
lib/timers.js | 8 +++----
test/parallel/test-timers-this.js | 37 +++++++++++++++++++++++++++++--
2 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/lib/timers.js b/lib/timers.js
index e3e64287509a30..c7bac4114bf339 100644
--- a/lib/timers.js
+++ b/lib/timers.js
@@ -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);
};
}
@@ -242,12 +242,12 @@ exports.setInterval = function(callback, repeat) {
function wrapper() {
callback.apply(this, args);
// If callback called clearInterval().
- if (timer._repeat === false) return;
+ if (this._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);
} else {
- timer._idleTimeout = repeat;
+ this._idleTimeout = repeat;
exports.active(timer);
}
}
@@ -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();
diff --git a/test/parallel/test-timers-this.js b/test/parallel/test-timers-this.js
index cacb3f247cc4b8..b79e55da8c3cde 100644
--- a/test/parallel/test-timers-this.js
+++ b/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);
});
From fe805ef7afa063a1a9c4309def73357501402d4d Mon Sep 17 00:00:00 2001
From: Paul Loyd <pavelko95@gmail.com>
Date: Tue, 13 Jan 2015 19:20:20 +0300
Subject: [PATCH 2/3] timers: updating `now` within `listOnTimeout`
See joyent/node#8105.
---
lib/timers.js | 8 ++++----
test/parallel/test-timers-busy.js | 27 +++++++++++++++++++++++++
test/parallel/test-timers-unref-call.js | 2 --
3 files changed, 31 insertions(+), 6 deletions(-)
create mode 100644 test/parallel/test-timers-busy.js
diff --git a/lib/timers.js b/lib/timers.js
index c7bac4114bf339..31f3c00c0a44a7 100644
--- a/lib/timers.js
+++ b/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);
diff --git a/test/parallel/test-timers-busy.js b/test/parallel/test-timers-busy.js
new file mode 100644
index 00000000000000..c4073a727588fb
--- /dev/null
+++ b/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();
diff --git a/test/parallel/test-timers-unref-call.js b/test/parallel/test-timers-unref-call.js
index 4f7865b8457601..b6ba09eed0db1e 100644
--- a/test/parallel/test-timers-unref-call.js
+++ b/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 };
From 1c9e35a1669ff48c34e04c681de2156aa107351d Mon Sep 17 00:00:00 2001
From: Paul Loyd <pavelko95@gmail.com>
Date: Tue, 13 Jan 2015 19:36:57 +0300
Subject: [PATCH 3/3] timers: now `setInterval` complies with DOM
See joyent/node#8066.
---
lib/timers.js | 6 ++--
src/timer_wrap.cc | 7 ++++
test/parallel/test-timers-intervals.js | 50 ++++++++++++++++++++++++++
3 files changed, 60 insertions(+), 3 deletions(-)
create mode 100644 test/parallel/test-timers-intervals.js
diff --git a/lib/timers.js b/lib/timers.js
index 31f3c00c0a44a7..8d75c8f921f11c 100644
--- a/lib/timers.js
+++ b/lib/timers.js
@@ -240,15 +240,15 @@ exports.setInterval = function(callback, repeat) {
return timer;
function wrapper() {
- callback.apply(this, args);
- // If callback called clearInterval().
- if (this._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 {
this._idleTimeout = repeat;
exports.active(timer);
+ callback.apply(this, args);
}
}
};
diff --git a/src/timer_wrap.cc b/src/timer_wrap.cc
index d71213f2202984..b52dfaca44af7c 100644
--- a/src/timer_wrap.cc
+++ b/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());
diff --git a/test/parallel/test-timers-intervals.js b/test/parallel/test-timers-intervals.js
new file mode 100644
index 00000000000000..a52a91126da3f7
--- /dev/null
+++ b/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
+ });
+ });
+});