From caf0b36de33ad4991e3f21ed088a84c68cb0662d Mon Sep 17 00:00:00 2001 From: Roman Reiss Date: Sat, 21 Mar 2015 17:39:35 +0100 Subject: [PATCH] timers: assure setTimeout callback only runs once Calling this.unref() during the callback of SetTimeout caused the callback to get executed twice because unref() didn't expect to be called during that time and did not stop the ref()ed Timeout but did start a new timer. This commit prevents the new timer creation when the callback was already called. Fixes: https://github.com/iojs/io.js/issues/1191 Reviewed-by: Trevor Norris Reviewed-By: Jeremiah Senkpiel PR-URL: https://github.com/iojs/io.js/pull/1231 --- lib/timers.js | 7 +++++++ test/parallel/test-timers-unref.js | 12 ++++++++++++ 2 files changed, 19 insertions(+) diff --git a/lib/timers.js b/lib/timers.js index 102a927a19c6a1..dd39e7b3d9edaf 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(); @@ -293,6 +294,7 @@ exports.clearInterval = function(timer) { const Timeout = function(after) { + this._called = false; this._idleTimeout = after; this._idlePrev = this; this._idleNext = this; @@ -310,6 +312,10 @@ Timeout.prototype.unref = function() { 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); @@ -492,6 +498,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..2f750228a4ef3f 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,16 @@ check_unref = setInterval(function() { checks += 1; }, 100); +setTimeout(function() { + unref_callbacks++; + this.unref(); +}, SHORT_TIME); + +// Should not timeout the test +setInterval(function() { + this.unref(); +}, SHORT_TIME); + // Should not assert on args.Holder()->InternalFieldCount() > 0. See #4261. (function() { var t = setInterval(function() {}, 1); @@ -46,4 +57,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'); });