diff --git a/lib/internal/timers.js b/lib/internal/timers.js index 1b6422f41ea7af..b62ab9499c8fff 100644 --- a/lib/internal/timers.js +++ b/lib/internal/timers.js @@ -155,7 +155,7 @@ function initAsyncResource(resource, type) { // Timer constructor function. // The entire prototype is defined in lib/timers.js -function Timeout(callback, after, args, isRepeat) { +function Timeout(callback, after, args, isRepeat, isRefed) { after *= 1; // Coalesce to number or NaN if (!(after >= 1 && after <= TIMEOUT_MAX)) { if (after > TIMEOUT_MAX) { @@ -179,7 +179,9 @@ function Timeout(callback, after, args, isRepeat) { this._repeat = isRepeat ? after : null; this._destroyed = false; - this[kRefed] = null; + if (isRefed) + incRefCount(); + this[kRefed] = isRefed; initAsyncResource(this, 'Timeout'); } @@ -207,21 +209,23 @@ Timeout.prototype.refresh = function() { Timeout.prototype.unref = function() { if (this[kRefed]) { this[kRefed] = false; - decRefCount(); + if (!this._destroyed) + decRefCount(); } return this; }; Timeout.prototype.ref = function() { - if (this[kRefed] === false) { + if (!this[kRefed]) { this[kRefed] = true; - incRefCount(); + if (!this._destroyed) + incRefCount(); } return this; }; Timeout.prototype.hasRef = function() { - return !!this[kRefed]; + return this[kRefed]; }; function TimersList(expiry, msecs) { @@ -295,27 +299,47 @@ function decRefCount() { // Schedule or re-schedule a timer. // The item must have been enroll()'d first. function active(item) { - insert(item, true, getLibuvNow()); + insertGuarded(item, true); } // Internal APIs that need timeouts should use `unrefActive()` instead of // `active()` so that they do not unnecessarily keep the process open. function unrefActive(item) { - insert(item, false, getLibuvNow()); + insertGuarded(item, false); } // The underlying logic for scheduling or re-scheduling a timer. // // Appends a timer onto the end of an existing timers list, or creates a new // list if one does not already exist for the specified timeout duration. -function insert(item, refed, start) { - let msecs = item._idleTimeout; +function insertGuarded(item, refed, start) { + const msecs = item._idleTimeout; if (msecs < 0 || msecs === undefined) return; - // Truncate so that accuracy of sub-millisecond timers is not assumed. - msecs = MathTrunc(msecs); + insert(item, msecs, start); + + const isDestroyed = item._destroyed; + if (isDestroyed || !item[async_id_symbol]) { + item._destroyed = false; + initAsyncResource(item, 'Timeout'); + } + + if (isDestroyed) { + if (refed) + incRefCount(); + } else if (refed === !item[kRefed]) { + if (refed) + incRefCount(); + else + decRefCount(); + } + item[kRefed] = refed; +} +function insert(item, msecs, start = getLibuvNow()) { + // Truncate so that accuracy of sub-milisecond timers is not assumed. + msecs = MathTrunc(msecs); item._idleStart = start; // Use an existing list if there is one, otherwise we need to make a new one. @@ -332,19 +356,6 @@ function insert(item, refed, start) { } } - if (!item[async_id_symbol] || item._destroyed) { - item._destroyed = false; - initAsyncResource(item, 'Timeout'); - } - - if (refed === !item[kRefed]) { - if (refed) - incRefCount(); - else - decRefCount(); - } - item[kRefed] = refed; - L.append(list, item); } @@ -354,8 +365,8 @@ function setUnrefTimeout(callback, after) { throw new ERR_INVALID_CALLBACK(callback); } - const timer = new Timeout(callback, after, undefined, false); - unrefActive(timer); + const timer = new Timeout(callback, after, undefined, false, false); + insert(timer, timer._idleTimeout); return timer; } @@ -514,13 +525,14 @@ function getTimerCallbacks(runNextTicks) { const asyncId = timer[async_id_symbol]; if (!timer._onTimeout) { - if (timer[kRefed]) - refCount--; - timer[kRefed] = null; - - if (destroyHooksExist() && !timer._destroyed) { - emitDestroy(asyncId); + if (!timer._destroyed) { timer._destroyed = true; + + if (timer[kRefed]) + refCount--; + + if (destroyHooksExist()) + emitDestroy(asyncId); } continue; } @@ -540,18 +552,15 @@ function getTimerCallbacks(runNextTicks) { } finally { if (timer._repeat && timer._idleTimeout !== -1) { timer._idleTimeout = timer._repeat; - if (start === undefined) - start = getLibuvNow(); - insert(timer, timer[kRefed], start); - } else if (!timer._idleNext && !timer._idlePrev) { + insert(timer, timer._idleTimeout, start); + } else if (!timer._idleNext && !timer._idlePrev && !timer._destroyed) { + timer._destroyed = true; + if (timer[kRefed]) refCount--; - timer[kRefed] = null; - if (destroyHooksExist() && !timer._destroyed) { - emitDestroy(timer[async_id_symbol]); - } - timer._destroyed = true; + if (destroyHooksExist()) + emitDestroy(asyncId); } } @@ -598,6 +607,7 @@ module.exports = { }, active, unrefActive, + insert, timerListMap, timerListQueue, decRefCount, diff --git a/lib/timers.js b/lib/timers.js index 68327e2c3e5947..3377bb6c785da7 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -45,7 +45,8 @@ const { timerListQueue, immediateQueue, active, - unrefActive + unrefActive, + insert } = require('internal/timers'); const { promisify: { custom: customPromisify }, @@ -62,13 +63,14 @@ const { // Remove a timer. Cancels the timeout and resets the relevant timer properties. function unenroll(item) { + if (item._destroyed) + return; + + item._destroyed = true; + // Fewer checks may be possible, but these cover everything. - if (destroyHooksExist() && - item[async_id_symbol] !== undefined && - !item._destroyed) { + if (destroyHooksExist() && item[async_id_symbol] !== undefined) emitDestroy(item[async_id_symbol]); - } - item._destroyed = true; L.remove(item); @@ -89,7 +91,6 @@ function unenroll(item) { decRefCount(); } - item[kRefed] = null; // If active is called later, then we want to make sure not to insert again item._idleTimeout = -1; @@ -141,8 +142,8 @@ function setTimeout(callback, after, arg1, arg2, arg3) { break; } - const timeout = new Timeout(callback, after, args, false); - active(timeout); + const timeout = new Timeout(callback, after, args, false, true); + insert(timeout, timeout._idleTimeout); return timeout; } @@ -150,7 +151,8 @@ function setTimeout(callback, after, arg1, arg2, arg3) { setTimeout[customPromisify] = function(after, value) { const args = value !== undefined ? [value] : value; return new Promise((resolve) => { - active(new Timeout(resolve, after, args, false)); + const timeout = new Timeout(resolve, after, args, false, true); + insert(timeout, timeout._idleTimeout); }); }; @@ -187,8 +189,8 @@ function setInterval(callback, repeat, arg1, arg2, arg3) { break; } - const timeout = new Timeout(callback, repeat, args, true); - active(timeout); + const timeout = new Timeout(callback, repeat, args, true, true); + insert(timeout, timeout._idleTimeout); return timeout; } diff --git a/test/parallel/test-timers-refresh.js b/test/parallel/test-timers-refresh.js index e186c63a65ecf9..a96f5ad5368275 100644 --- a/test/parallel/test-timers-refresh.js +++ b/test/parallel/test-timers-refresh.js @@ -72,6 +72,20 @@ const { inspect } = require('util'); strictEqual(timer.refresh(), timer); } +// regular timer +{ + let called = false; + const timer = setTimeout(common.mustCall(() => { + if (!called) { + called = true; + process.nextTick(common.mustCall(() => { + timer.refresh(); + strictEqual(timer.hasRef(), true); + })); + } + }, 2), 1); +} + // interval { let called = 0;