1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

timers: assure setTimeout callback only runs once

Browse Source 
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 <trev.norris@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
PR-URL: https://github.com/iojs/io.js/pull/1231
This commit is contained in:
Roman Reiss 2015-03-21 17:39:35 +01:00
parent 2ccc8f3970
commit caf0b36de3
2 changed files with 19 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
lib/timers.js
View File

@ -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)

12
test/parallel/test-timers-unref.js
View File

@ -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');
});