Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
timers: fix refresh for expired timers
Expired timers were not being refresh correctly and
would always act as unrefed if refresh was called.

PR-URL: #27345
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
apapirovski authored and BridgeAR committed Jan 3, 2020
1 parent 83330a0 commit fe4f55e
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 23 deletions.
40 changes: 23 additions & 17 deletions lib/internal/timers.js
Expand Up @@ -209,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) {
Expand Down Expand Up @@ -317,12 +319,16 @@ function insertGuarded(item, refed, start) {

insert(item, msecs, start);

if (!item[async_id_symbol] || item._destroyed) {
const isDestroyed = item._destroyed;
if (isDestroyed || !item[async_id_symbol]) {
item._destroyed = false;
initAsyncResource(item, 'Timeout');
}

if (refed === !item[kRefed]) {
if (isDestroyed) {
if (refed)
incRefCount();
} else if (refed === !item[kRefed]) {
if (refed)
incRefCount();
else
Expand Down Expand Up @@ -519,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;
}
Expand All @@ -546,15 +553,14 @@ function getTimerCallbacks(runNextTicks) {
if (timer._repeat && timer._idleTimeout !== -1) {
timer._idleTimeout = timer._repeat;
insert(timer, timer._idleTimeout, start);
} else if (!timer._idleNext && !timer._idlePrev) {
} else if (!timer._idleNext && !timer._idlePrev && !timer._destroyed) {
timer._destroyed = true;

if (timer[kRefed])
refCount--;
timer[kRefed] = null;

if (destroyHooksExist() && !timer._destroyed) {
if (destroyHooksExist())
emitDestroy(asyncId);
}
timer._destroyed = true;
}
}

Expand Down
12 changes: 6 additions & 6 deletions lib/timers.js
Expand Up @@ -64,13 +64,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);

Expand All @@ -91,7 +92,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;
Expand Down
14 changes: 14 additions & 0 deletions test/parallel/test-timers-refresh.js
Expand Up @@ -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;
Expand Down

0 comments on commit fe4f55e

Please sign in to comment.