diff --git a/lib/timers.js b/lib/timers.js index 668d5536c8186c..27c4b0717653ca 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -157,15 +157,8 @@ exports.enroll = function(item, msecs) { // it will reset its timeout. exports.active = function(item) { var msecs = item._idleTimeout; - if (msecs >= 0) { - var list = lists[msecs]; - if (!list || L.isEmpty(list)) { - insert(item, msecs); - } else { - item._idleStart = Timer.now(); - L.append(list, item); - } - } + if (msecs >= 0) + insert(item, msecs); }; @@ -272,6 +265,11 @@ exports.setInterval = function(callback, repeat) { function wrapper() { timer._repeat.call(this); + + // Timer might be closed - no point in restarting it + if (!timer._repeat) + return; + // If timer is unref'd (or was - it's permanently removed from the list.) if (this._handle) { this._handle.start(repeat, 0); @@ -301,6 +299,14 @@ const Timeout = function(after) { this._repeat = null; }; + +function unrefdHandle() { + this.owner._onTimeout(); + if (!this.owner._repeat) + this.owner.close(); +} + + Timeout.prototype.unref = function() { if (this._handle) { this._handle.unref(); @@ -315,7 +321,8 @@ Timeout.prototype.unref = function() { if (this._called && !this._repeat) return; this._handle = new Timer(); - this._handle[kOnTimeout] = this._onTimeout; + this._handle.owner = this; + this._handle[kOnTimeout] = unrefdHandle; this._handle.start(delay, 0); this._handle.domain = this.domain; this._handle.unref(); diff --git a/src/timer_wrap.cc b/src/timer_wrap.cc index d71213f2202984..f65290a5162398 100644 --- a/src/timer_wrap.cc +++ b/src/timer_wrap.cc @@ -73,6 +73,8 @@ class TimerWrap : public HandleWrap { static void Start(const FunctionCallbackInfo& args) { TimerWrap* wrap = Unwrap(args.Holder()); + CHECK(HandleWrap::IsAlive(wrap)); + int64_t timeout = args[0]->IntegerValue(); int64_t repeat = args[1]->IntegerValue(); int err = uv_timer_start(&wrap->handle_, OnTimeout, timeout, repeat); @@ -82,6 +84,8 @@ class TimerWrap : public HandleWrap { static void Stop(const FunctionCallbackInfo& args) { TimerWrap* wrap = Unwrap(args.Holder()); + CHECK(HandleWrap::IsAlive(wrap)); + int err = uv_timer_stop(&wrap->handle_); args.GetReturnValue().Set(err); } @@ -89,6 +93,8 @@ class TimerWrap : public HandleWrap { static void Again(const FunctionCallbackInfo& args) { TimerWrap* wrap = Unwrap(args.Holder()); + CHECK(HandleWrap::IsAlive(wrap)); + int err = uv_timer_again(&wrap->handle_); args.GetReturnValue().Set(err); } @@ -96,6 +102,8 @@ class TimerWrap : public HandleWrap { static void SetRepeat(const FunctionCallbackInfo& args) { TimerWrap* wrap = Unwrap(args.Holder()); + CHECK(HandleWrap::IsAlive(wrap)); + int64_t repeat = args[0]->IntegerValue(); uv_timer_set_repeat(&wrap->handle_, repeat); args.GetReturnValue().Set(0); @@ -104,6 +112,8 @@ class TimerWrap : public HandleWrap { static void GetRepeat(const FunctionCallbackInfo& args) { TimerWrap* wrap = Unwrap(args.Holder()); + CHECK(HandleWrap::IsAlive(wrap)); + int64_t repeat = uv_timer_get_repeat(&wrap->handle_); if (repeat <= 0xfffffff) args.GetReturnValue().Set(static_cast(repeat)); diff --git a/test/parallel/test-timers-unref-leak.js b/test/parallel/test-timers-unref-leak.js new file mode 100644 index 00000000000000..c8f958a47c40d2 --- /dev/null +++ b/test/parallel/test-timers-unref-leak.js @@ -0,0 +1,25 @@ +var assert = require('assert'); + +var called = 0; +var closed = 0; + +var timeout = setTimeout(function() { + called++; +}, 10); +timeout.unref(); + +// Wrap `close` method to check if the handle was closed +var close = timeout._handle.close; +timeout._handle.close = function() { + closed++; + return close.apply(this, arguments); +}; + +// Just to keep process alive and let previous timer's handle die +setTimeout(function() { +}, 50); + +process.on('exit', function() { + assert.equal(called, 1); + assert.equal(closed, 1); +}); diff --git a/test/parallel/test-timers-unrefd-interval-still-fires.js b/test/parallel/test-timers-unrefd-interval-still-fires.js new file mode 100644 index 00000000000000..3ea94454cfdb49 --- /dev/null +++ b/test/parallel/test-timers-unrefd-interval-still-fires.js @@ -0,0 +1,18 @@ +/* + * This test is a regression test for joyent/node#8900. + */ +var assert = require('assert'); + +var N = 5; +var nbIntervalFired = 0; +var timer = setInterval(function() { + ++nbIntervalFired; + if (nbIntervalFired === N) + clearInterval(timer); +}, 1); + +timer.unref(); + +setTimeout(function onTimeout() { + assert.strictEqual(nbIntervalFired, N); +}, 100);