Commit f86862a
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>1 parent a4cbd57 commit f86862a
3 files changed
Lines changed: 43 additions & 23 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -209,21 +209,23 @@ Timeout.prototype.refresh = function() { | |||
| 209 | 209 | Timeout.prototype.unref = function() { | |
| 210 | 210 | if (this[kRefed]) { | |
| 211 | 211 | this[kRefed] = false; | |
| 212 | - decRefCount(); | ||
| 212 | + if (!this._destroyed) | ||
| 213 | + decRefCount(); | ||
| 213 | 214 | } | |
| 214 | 215 | return this; | |
| 215 | 216 | }; | |
| 216 | 217 | ||
| 217 | 218 | Timeout.prototype.ref = function() { | |
| 218 | - if (this[kRefed] === false) { | ||
| 219 | + if (!this[kRefed]) { | ||
| 219 | 220 | this[kRefed] = true; | |
| 220 | - incRefCount(); | ||
| 221 | + if (!this._destroyed) | ||
| 222 | + incRefCount(); | ||
| 221 | 223 | } | |
| 222 | 224 | return this; | |
| 223 | 225 | }; | |
| 224 | 226 | ||
| 225 | 227 | Timeout.prototype.hasRef = function() { | |
| 226 | - return !!this[kRefed]; | ||
| 228 | + return this[kRefed]; | ||
| 227 | 229 | }; | |
| 228 | 230 | ||
| 229 | 231 | function TimersList(expiry, msecs) { | |
@@ -317,12 +319,16 @@ function insertGuarded(item, refed, start) { | |||
| 317 | 319 | ||
| 318 | 320 | insert(item, msecs, start); | |
| 319 | 321 | ||
| 320 | - if (!item[async_id_symbol] || item._destroyed) { | ||
| 322 | + const isDestroyed = item._destroyed; | ||
| 323 | + if (isDestroyed || !item[async_id_symbol]) { | ||
| 321 | 324 | item._destroyed = false; | |
| 322 | 325 | initAsyncResource(item, 'Timeout'); | |
| 323 | 326 | } | |
| 324 | 327 | ||
| 325 | - if (refed === !item[kRefed]) { | ||
| 328 | + if (isDestroyed) { | ||
| 329 | + if (refed) | ||
| 330 | + incRefCount(); | ||
| 331 | + } else if (refed === !item[kRefed]) { | ||
| 326 | 332 | if (refed) | |
| 327 | 333 | incRefCount(); | |
| 328 | 334 | else | |
@@ -519,13 +525,14 @@ function getTimerCallbacks(runNextTicks) { | |||
| 519 | 525 | const asyncId = timer[async_id_symbol]; | |
| 520 | 526 | ||
| 521 | 527 | 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) { | ||
| 528 | 529 | timer._destroyed = true; | |
| 530 | + | ||
| 531 | + if (timer[kRefed]) | ||
| 532 | + refCount--; | ||
| 533 | + | ||
| 534 | + if (destroyHooksExist()) | ||
| 535 | + emitDestroy(asyncId); | ||
| 529 | 536 | } | |
| 530 | 537 | continue; | |
| 531 | 538 | } | |
@@ -546,15 +553,14 @@ function getTimerCallbacks(runNextTicks) { | |||
| 546 | 553 | if (timer._repeat && timer._idleTimeout !== -1) { | |
| 547 | 554 | timer._idleTimeout = timer._repeat; | |
| 548 | 555 | 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 | + | ||
| 550 | 559 | if (timer[kRefed]) | |
| 551 | 560 | refCount--; | |
| 552 | - timer[kRefed] = null; | ||
| 553 | 561 | ||
| 554 | - if (destroyHooksExist() && !timer._destroyed) { | ||
| 562 | + if (destroyHooksExist()) | ||
| 555 | 563 | emitDestroy(asyncId); | |
| 556 | - } | ||
| 557 | - timer._destroyed = true; | ||
| 558 | 564 | } | |
| 559 | 565 | } | |
| 560 | 566 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -64,13 +64,14 @@ const { | |||
| 64 | 64 | ||
| 65 | 65 | // Remove a timer. Cancels the timeout and resets the relevant timer properties. | |
| 66 | 66 | function unenroll(item) { | |
| 67 | + if (item._destroyed) | ||
| 68 | + return; | ||
| 69 | + | ||
| 70 | + item._destroyed = true; | ||
| 71 | + | ||
| 67 | 72 | // 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) | ||
| 71 | 74 | emitDestroy(item[async_id_symbol]); | |
| 72 | - } | ||
| 73 | - item._destroyed = true; | ||
| 74 | 75 | ||
| 75 | 76 | L.remove(item); | |
| 76 | 77 | ||
@@ -91,7 +92,6 @@ function unenroll(item) { | |||
| 91 | 92 | ||
| 92 | 93 | decRefCount(); | |
| 93 | 94 | } | |
| 94 | - item[kRefed] = null; | ||
| 95 | 95 | ||
| 96 | 96 | // If active is called later, then we want to make sure not to insert again | |
| 97 | 97 | item._idleTimeout = -1; | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -72,6 +72,20 @@ const { inspect } = require('util'); | |||
| 72 | 72 | strictEqual(timer.refresh(), timer); | |
| 73 | 73 | } | |
| 74 | 74 | ||
| 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 | + | ||
| 75 | 89 | // interval | |
| 76 | 90 | { | |
| 77 | 91 | let called = 0; | |
0 commit comments