From 951168f96c6656d4c7a05dfd63e001badd4dfdfe Mon Sep 17 00:00:00 2001
From: Roman Reiss <me@silverwind.io>
Date: Sat, 21 Mar 2015 17:39:35 +0100
Subject: [PATCH 1/5] timers: assure setTimeout callback only runs once
This fixes an edge case where running this.unref() during the callback caused the
callback to get executed multiple times.
---
lib/timers.js | 5 +++++
test/parallel/test-timers-unref.js | 7 +++++++
2 files changed, 12 insertions(+)
diff --git a/lib/timers.js b/lib/timers.js
index 102a927a19c6a1..dfba03d97923e7 100644
--- a/lib/timers.js
+++ b/lib/timers.js
@@ -85,6 +85,7 @@ function listOnTimeout() {
if (domain)
domain.enter();
threw = true;
+ first._called = true;
first._onTimeout();
if (domain)
domain.exit();
@@ -305,6 +306,9 @@ Timeout.prototype.unref = function() {
if (this._handle) {
this._handle.unref();
} else if (typeof(this._onTimeout) === 'function') {
+ // Prevent running callback multiple times
+ // when unref() is called during the callback
+ if (this._called) return;
var now = Timer.now();
if (!this._idleStart) this._idleStart = now;
var delay = this._idleStart + this._idleTimeout - now;
@@ -492,6 +496,7 @@ function unrefTimeout() {
if (domain) domain.enter();
threw = true;
debug('unreftimer firing timeout');
+ first._called = true;
first._onTimeout();
threw = false;
if (domain)
diff --git a/test/parallel/test-timers-unref.js b/test/parallel/test-timers-unref.js
index 9434dbbd36ad78..23d4fc4cc5f953 100644
--- a/test/parallel/test-timers-unref.js
+++ b/test/parallel/test-timers-unref.js
@@ -5,6 +5,7 @@ var interval_fired = false,
timeout_fired = false,
unref_interval = false,
unref_timer = false,
+ unref_callbacks = 0,
interval, check_unref, checks = 0;
var LONG_TIME = 10 * 1000;
@@ -34,6 +35,11 @@ check_unref = setInterval(function() {
checks += 1;
}, 100);
+setTimeout(function() {
+ unref_callbacks++;
+ this.unref();
+}, SHORT_TIME);
+
// Should not assert on args.Holder()->InternalFieldCount() > 0. See #4261.
(function() {
var t = setInterval(function() {}, 1);
@@ -46,4 +52,5 @@ process.on('exit', function() {
assert.strictEqual(timeout_fired, false, 'Timeout should not fire');
assert.strictEqual(unref_timer, true, 'An unrefd timeout should still fire');
assert.strictEqual(unref_interval, true, 'An unrefd interval should still fire');
+ assert.strictEqual(unref_callbacks, 1, 'Callback should only run once');
});
From bc94f6c3adf0b8a96acf5c1f04d22a7d0fe892cf Mon Sep 17 00:00:00 2001
From: Roman Reiss <me@silverwind.io>
Date: Wed, 25 Mar 2015 19:19:29 +0100
Subject: [PATCH 2/5] Add _called to prototype
---
lib/timers.js | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/timers.js b/lib/timers.js
index dfba03d97923e7..1ea00d7ce44edd 100644
--- a/lib/timers.js
+++ b/lib/timers.js
@@ -294,6 +294,7 @@ exports.clearInterval = function(timer) {
const Timeout = function(after) {
+ this._called = false;
this._idleTimeout = after;
this._idlePrev = this;
this._idleNext = this;
From a58d1492d564a78b6bfcd4cf1738035029a797f2 Mon Sep 17 00:00:00 2001
From: Roman Reiss <me@silverwind.io>
Date: Wed, 25 Mar 2015 19:55:07 +0100
Subject: [PATCH 3/5] Add test for unref during setInterval
---
test/parallel/test-timers-unref.js | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/test/parallel/test-timers-unref.js b/test/parallel/test-timers-unref.js
index 23d4fc4cc5f953..69a67bddc3ff55 100644
--- a/test/parallel/test-timers-unref.js
+++ b/test/parallel/test-timers-unref.js
@@ -35,10 +35,14 @@ check_unref = setInterval(function() {
checks += 1;
}, 100);
+// unref() during the callback
setTimeout(function() {
unref_callbacks++;
this.unref();
}, SHORT_TIME);
+setInterval(function() {
+ this.unref();
+}, SHORT_TIME);
// Should not assert on args.Holder()->InternalFieldCount() > 0. See #4261.
(function() {
From c99318c733f8c4b6bb4cd94afd5bb3d555f5fde2 Mon Sep 17 00:00:00 2001
From: Roman Reiss <me@silverwind.io>
Date: Wed, 25 Mar 2015 20:44:15 +0100
Subject: [PATCH 4/5] Let timer unenroll and prevent setInterval issues
---
lib/timers.js | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/lib/timers.js b/lib/timers.js
index 1ea00d7ce44edd..dd39e7b3d9edaf 100644
--- a/lib/timers.js
+++ b/lib/timers.js
@@ -307,14 +307,15 @@ Timeout.prototype.unref = function() {
if (this._handle) {
this._handle.unref();
} else if (typeof(this._onTimeout) === 'function') {
- // Prevent running callback multiple times
- // when unref() is called during the callback
- if (this._called) return;
var now = Timer.now();
if (!this._idleStart) this._idleStart = now;
var delay = this._idleStart + this._idleTimeout - now;
if (delay < 0) delay = 0;
exports.unenroll(this);
+
+ // Prevent running cb again when unref() is called during the same cb
+ if (this._called && !this._repeat) return;
+
this._handle = new Timer();
this._handle[kOnTimeout] = this._onTimeout;
this._handle.start(delay, 0);
From 1a824b17b12a2dff30089c77b322285e22afae5f Mon Sep 17 00:00:00 2001
From: Roman Reiss <me@silverwind.io>
Date: Wed, 25 Mar 2015 21:22:24 +0100
Subject: [PATCH 5/5] Add note to setInterval test
---
test/parallel/test-timers-unref.js | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/test/parallel/test-timers-unref.js b/test/parallel/test-timers-unref.js
index 69a67bddc3ff55..2f750228a4ef3f 100644
--- a/test/parallel/test-timers-unref.js
+++ b/test/parallel/test-timers-unref.js
@@ -35,11 +35,12 @@ check_unref = setInterval(function() {
checks += 1;
}, 100);
-// unref() during the callback
setTimeout(function() {
unref_callbacks++;
this.unref();
}, SHORT_TIME);
+
+// Should not timeout the test
setInterval(function() {
this.unref();
}, SHORT_TIME);