diff --git a/lib/timers.js b/lib/timers.js index 78953f25d5a17e..56af3a92d6bfe2 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -384,6 +384,9 @@ function ontimeout(timer) { function rearm(timer) { + // // Do not re-arm unenroll'd or closed timers. + if (timer._idleTimeout === -1) return; + // If timer is unref'd (or was - it's permanently removed from the list.) if (timer._handle && timer instanceof Timeout) { timer._handle.start(timer._repeat); @@ -463,9 +466,17 @@ function Timeout(after, callback, args) { function unrefdHandle() { - ontimeout(this.owner); - if (!this.owner._repeat) + // Don't attempt to call the callback if it is not a function. + if (typeof this.owner._onTimeout === 'function') { + ontimeout(this.owner); + } + + // Make sure we clean up if the callback is no longer a function + // even if the timer is an interval. + if (!this.owner._repeat + || typeof this.owner._onTimeout !== 'function') { this.owner.close(); + } } @@ -505,6 +516,7 @@ Timeout.prototype.ref = function() { Timeout.prototype.close = function() { this._onTimeout = null; if (this._handle) { + this._idleTimeout = -1; this._handle[kOnTimeout] = null; this._handle.close(); } else { diff --git a/test/parallel/test-timers-unenroll-unref-interval.js b/test/parallel/test-timers-unenroll-unref-interval.js new file mode 100644 index 00000000000000..2c8a6a385d2304 --- /dev/null +++ b/test/parallel/test-timers-unenroll-unref-interval.js @@ -0,0 +1,49 @@ +'use strict'; + +const common = require('../common'); +const timers = require('timers'); + +{ + const interval = setInterval(common.mustCall(() => { + clearTimeout(interval); + }), 1).unref(); +} + +{ + const interval = setInterval(common.mustCall(() => { + interval.close(); + }), 1).unref(); +} + +{ + const interval = setInterval(common.mustCall(() => { + timers.unenroll(interval); + }), 1).unref(); +} + +{ + const interval = setInterval(common.mustCall(() => { + interval._idleTimeout = -1; + }), 1).unref(); +} + +{ + const interval = setInterval(common.mustCall(() => { + interval._onTimeout = null; + }), 1).unref(); +} + +// Use timers' intrinsic behavior to keep this open +// exactly long enough for the problem to manifest. +// +// See https://github.com/nodejs/node/issues/9561 +// +// Since this is added after it will always fire later +// than the previous timeouts, unrefed or not. +// +// Keep the event loop alive for one timeout and then +// another. Any problems will occur when the second +// should be called but before it is able to be. +setTimeout(common.mustCall(() => { + setTimeout(common.mustCall(() => {}), 1); +}), 1);