Skip to content

Commit e5c68cd

Browse files
apapirovskitargos
authored andcommitted
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 <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent facee2a commit e5c68cd

File tree

3 files changed

+43
-23
lines changed

3 files changed

+43
-23
lines changed

lib/internal/timers.js

+23-17
Original file line numberDiff line numberDiff line change
@@ -209,21 +209,23 @@ Timeout.prototype.refresh = function() {
209209
Timeout.prototype.unref = function() {
210210
if (this[kRefed]) {
211211
this[kRefed] = false;
212-
decRefCount();
212+
if (!this._destroyed)
213+
decRefCount();
213214
}
214215
return this;
215216
};
216217

217218
Timeout.prototype.ref = function() {
218-
if (this[kRefed] === false) {
219+
if (!this[kRefed]) {
219220
this[kRefed] = true;
220-
incRefCount();
221+
if (!this._destroyed)
222+
incRefCount();
221223
}
222224
return this;
223225
};
224226

225227
Timeout.prototype.hasRef = function() {
226-
return !!this[kRefed];
228+
return this[kRefed];
227229
};
228230

229231
function TimersList(expiry, msecs) {
@@ -317,12 +319,16 @@ function insertGuarded(item, refed, start) {
317319

318320
insert(item, msecs, start);
319321

320-
if (!item[async_id_symbol] || item._destroyed) {
322+
const isDestroyed = item._destroyed;
323+
if (isDestroyed || !item[async_id_symbol]) {
321324
item._destroyed = false;
322325
initAsyncResource(item, 'Timeout');
323326
}
324327

325-
if (refed === !item[kRefed]) {
328+
if (isDestroyed) {
329+
if (refed)
330+
incRefCount();
331+
} else if (refed === !item[kRefed]) {
326332
if (refed)
327333
incRefCount();
328334
else
@@ -519,13 +525,14 @@ function getTimerCallbacks(runNextTicks) {
519525
const asyncId = timer[async_id_symbol];
520526

521527
if (!timer._onTimeout) {
522-
if (timer[kRefed])
523-
refCount--;
524-
timer[kRefed] = null;
525-
526-
if (destroyHooksExist() && !timer._destroyed) {
527-
emitDestroy(asyncId);
528+
if (!timer._destroyed) {
528529
timer._destroyed = true;
530+
531+
if (timer[kRefed])
532+
refCount--;
533+
534+
if (destroyHooksExist())
535+
emitDestroy(asyncId);
529536
}
530537
continue;
531538
}
@@ -546,15 +553,14 @@ function getTimerCallbacks(runNextTicks) {
546553
if (timer._repeat && timer._idleTimeout !== -1) {
547554
timer._idleTimeout = timer._repeat;
548555
insert(timer, timer._idleTimeout, start);
549-
} else if (!timer._idleNext && !timer._idlePrev) {
556+
} else if (!timer._idleNext && !timer._idlePrev && !timer._destroyed) {
557+
timer._destroyed = true;
558+
550559
if (timer[kRefed])
551560
refCount--;
552-
timer[kRefed] = null;
553561

554-
if (destroyHooksExist() && !timer._destroyed) {
562+
if (destroyHooksExist())
555563
emitDestroy(asyncId);
556-
}
557-
timer._destroyed = true;
558564
}
559565
}
560566

lib/timers.js

+6-6
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,14 @@ const {
6464

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

7576
L.remove(item);
7677

@@ -91,7 +92,6 @@ function unenroll(item) {
9192

9293
decRefCount();
9394
}
94-
item[kRefed] = null;
9595

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

test/parallel/test-timers-refresh.js

+14
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)