timers: fix setInterval() assert

Test case:

  var t = setInterval(function() {}, 1);
  process.nextTick(t.unref);

Output:

  Assertion failed: (args.Holder()->InternalFieldCount() > 0),
  function Unref, file ../src/handle_wrap.cc, line 78.

setInterval() returns a binding layer object. Make it stop doing that,
wrap the raw process.binding('timer_wrap').Timer object in a Timeout
object.

Fixes #4261.
pull/5010/head
Ben Noordhuis 2013-03-21 12:19:03 +01:00
parent 1deeab29f2
commit 22533c035d
2 changed files with 32 additions and 12 deletions

View File

@ -222,7 +222,7 @@ exports.setTimeout = function(callback, after) {
exports.clearTimeout = function(timer) {
if (timer && (timer.ontimeout || timer._onTimeout)) {
timer.ontimeout = timer._onTimeout = null;
if (timer instanceof Timer || timer instanceof Timeout) {
if (timer instanceof Timeout) {
timer.close(); // for after === 0
} else {
exports.unenroll(timer);
@ -232,39 +232,52 @@ exports.clearTimeout = function(timer) {
exports.setInterval = function(callback, repeat) {
var timer = new Timer();
if (process.domain) timer.domain = process.domain;
repeat *= 1; // coalesce to number or NaN
if (!(repeat >= 1 && repeat <= TIMEOUT_MAX)) {
repeat = 1; // schedule on next tick, follows browser behaviour
}
var timer = new Timeout(repeat);
var args = Array.prototype.slice.call(arguments, 2);
timer.ontimeout = function() {
callback.apply(timer, args);
}
timer._onTimeout = wrapper;
timer._repeat = true;
if (process.domain) timer.domain = process.domain;
exports.active(timer);
timer.start(repeat, 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);
} else {
timer._idleTimeout = repeat;
exports.active(timer);
}
}
};
exports.clearInterval = function(timer) {
if (timer instanceof Timer) {
timer.ontimeout = null;
timer.close();
if (timer && timer._repeat) {
timer._repeat = false;
clearTimeout(timer);
}
};
var Timeout = function(after) {
this._idleTimeout = after;
this._idlePrev = this;
this._idleNext = this;
this._idleStart = null;
this._onTimeout = null;
this._repeat = false;
};
Timeout.prototype.unref = function() {

View File

@ -54,6 +54,13 @@ check_unref = setInterval(function() {
checks += 1;
}, 100);
// Should not assert on args.Holder()->InternalFieldCount() > 0. See #4261.
(function() {
var t = setInterval(function() {}, 1);
process.nextTick(t.unref.bind({}));
process.nextTick(t.unref.bind(t));
})();
process.on('exit', function() {
assert.strictEqual(interval_fired, false, 'Interval should not fire');
assert.strictEqual(timeout_fired, false, 'Timeout should not fire');