Skip to content

Commit 33b3eb4

Browse files
committed
timers: fix refresh for expired timers
Expired timers were not being refresh correctly and would always act as unrefed if refresh was called.
1 parent 21190b5 commit 33b3eb4

File tree

3 files changed

+40
-22
lines changed

3 files changed

+40
-22
lines changed

lib/internal/timers.js

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -207,23 +207,23 @@ Timeout.prototype.refresh = function() {
207207
};
208208

209209
Timeout.prototype.unref = function() {
210-
if (this[kRefed]) {
210+
if (this[kRefed] && !this._destroyed) {
211211
this[kRefed] = false;
212212
decRefCount();
213213
}
214214
return this;
215215
};
216216

217217
Timeout.prototype.ref = function() {
218-
if (this[kRefed] === false) {
218+
if (!this[kRefed] && !this._destroyed) {
219219
this[kRefed] = true;
220220
incRefCount();
221221
}
222222
return this;
223223
};
224224

225225
Timeout.prototype.hasRef = function() {
226-
return !!this[kRefed];
226+
return !!(this[kRefed] && !this._destroyed);
227227
};
228228

229229
function TimersList(expiry, msecs) {
@@ -317,12 +317,16 @@ function insertGuarded(item, refed, start) {
317317

318318
insert(item, msecs, start);
319319

320-
if (!item[async_id_symbol] || item._destroyed) {
320+
const isDestroyed = item._destroyed;
321+
if (isDestroyed || !item[async_id_symbol]) {
321322
item._destroyed = false;
322323
initAsyncResource(item, 'Timeout');
323324
}
324325

325-
if (refed === !item[kRefed]) {
326+
if (isDestroyed) {
327+
if (refed)
328+
incRefCount();
329+
} else if (refed === !item[kRefed]) {
326330
if (refed)
327331
incRefCount();
328332
else
@@ -519,13 +523,14 @@ function getTimerCallbacks(runNextTicks) {
519523
const asyncId = timer[async_id_symbol];
520524

521525
if (!timer._onTimeout) {
522-
if (timer[kRefed])
523-
refCount--;
524-
timer[kRefed] = null;
525-
526-
if (destroyHooksExist() && !timer._destroyed) {
527-
emitDestroy(asyncId);
526+
if (!timer._destroyed) {
528527
timer._destroyed = true;
528+
529+
if (timer[kRefed])
530+
refCount--;
531+
532+
if (destroyHooksExist())
533+
emitDestroy(asyncId);
529534
}
530535
continue;
531536
}
@@ -546,15 +551,14 @@ function getTimerCallbacks(runNextTicks) {
546551
if (timer._repeat && timer._idleTimeout !== -1) {
547552
timer._idleTimeout = timer._repeat;
548553
insert(timer, timer._idleTimeout, start);
549-
} else if (!timer._idleNext && !timer._idlePrev) {
554+
} else if (!timer._idleNext && !timer._idlePrev && !timer._destroyed) {
555+
timer._destroyed = true;
556+
550557
if (timer[kRefed])
551558
refCount--;
552-
timer[kRefed] = null;
553559

554-
if (destroyHooksExist() && !timer._destroyed) {
560+
if (destroyHooksExist())
555561
emitDestroy(asyncId);
556-
}
557-
timer._destroyed = true;
558562
}
559563
}
560564

lib/timers.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,14 @@ const {
6363

6464
// Remove a timer. Cancels the timeout and resets the relevant timer properties.
6565
function unenroll(item) {
66+
if (item._destroyed)
67+
return;
68+
69+
item._destroyed = true;
70+
6671
// Fewer checks may be possible, but these cover everything.
67-
if (destroyHooksExist() &&
68-
item[async_id_symbol] !== undefined &&
69-
!item._destroyed) {
72+
if (destroyHooksExist() && item[async_id_symbol] !== undefined)
7073
emitDestroy(item[async_id_symbol]);
71-
}
72-
item._destroyed = true;
7374

7475
L.remove(item);
7576

@@ -90,7 +91,6 @@ function unenroll(item) {
9091

9192
decRefCount();
9293
}
93-
item[kRefed] = null;
9494

9595
// If active is called later, then we want to make sure not to insert again
9696
item._idleTimeout = -1;

test/parallel/test-timers-refresh.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,20 @@ const { inspect } = require('util');
7272
strictEqual(timer.refresh(), timer);
7373
}
7474

75+
// regular timer
76+
{
77+
let called = false;
78+
const timer = setTimeout(common.mustCall(() => {
79+
if (!called) {
80+
called = true;
81+
process.nextTick(common.mustCall(() => {
82+
timer.refresh();
83+
strictEqual(timer.hasRef(), true);
84+
}));
85+
}
86+
}, 2), 1);
87+
}
88+
7589
// interval
7690
{
7791
let called = 0;

0 commit comments

Comments
 (0)