Skip to content

Commit

Permalink
timers: fix refresh for expired timers
Browse files Browse the repository at this point in the history
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 BethGriggs committed Feb 6, 2020
1 parent a4cbd57 commit f86862a
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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 f86862a

Please sign in to comment.