GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — timers: fix a bug in error handling · nodejs/node@be3c8aa

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
  • lib
    • timers.js
  • test/parallel
    • test-timers-throw-reschedule.js
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎lib/timers.js‎

Copy file name to clipboardExpand all lines: lib/timers.js
+37-23Lines changed: 37 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,17 @@ function TimersList(msecs, unrefed) {
217217
this.nextTick = false;
218218
}
219219

220+
function deleteTimersList(list, msecs) {
221+
// Either refedLists[msecs] or unrefedLists[msecs] may have been removed and
222+
// recreated since the reference to `list` was created. Make sure they're
223+
// the same instance of the list before destroying.
224+
if (list._unrefed === true && list === unrefedLists[msecs]) {
225+
delete unrefedLists[msecs];
226+
} else if (list === refedLists[msecs]) {
227+
delete refedLists[msecs];
228+
}
229+
}
230+
220231
function listOnTimeout() {
221232
var list = this._list;
222233
var msecs = list.msecs;
@@ -288,14 +299,7 @@ function listOnTimeout() {
288299
debug('%d list empty', msecs);
289300
assert(L.isEmpty(list));
290301

291-
// Either refedLists[msecs] or unrefedLists[msecs] may have been removed and
292-
// recreated since the reference to `list` was created. Make sure they're
293-
// the same instance of the list before destroying.
294-
if (list._unrefed === true && list === unrefedLists[msecs]) {
295-
delete unrefedLists[msecs];
296-
} else if (list === refedLists[msecs]) {
297-
delete refedLists[msecs];
298-
}
302+
deleteTimersList(list, msecs);
299303

300304
// Do not close the underlying handle if its ownership has changed
301305
// (e.g it was unrefed in its callback).
@@ -329,24 +333,34 @@ function tryOnTimeout(timer, list) {
329333
}
330334
}
331335

332-
if (!threw) return;
336+
if (threw) {
337+
const { msecs } = list;
338+
339+
if (L.isEmpty(list)) {
340+
deleteTimersList(list, msecs);
333341

334-
// Postpone all later list events to next tick. We need to do this
335-
// so that the events are called in the order they were created.
336-
const lists = list._unrefed === true ? unrefedLists : refedLists;
337-
for (var key in lists) {
338-
if (key > list.msecs) {
339-
lists[key].nextTick = true;
342+
if (!list._timer.owner)
343+
list._timer.close();
344+
} else {
345+
// Postpone all later list events to next tick. We need to do this
346+
// so that the events are called in the order they were created.
347+
const lists = list._unrefed === true ? unrefedLists : refedLists;
348+
for (var key in lists) {
349+
if (key > msecs) {
350+
lists[key].nextTick = true;
351+
}
352+
}
353+
354+
// We need to continue processing after domain error handling
355+
// is complete, but not by using whatever domain was left over
356+
// when the timeout threw its exception.
357+
const domain = process.domain;
358+
process.domain = null;
359+
// If we threw, we need to process the rest of the list in nextTick.
360+
process.nextTick(listOnTimeoutNT, list);
361+
process.domain = domain;
340362
}
341363
}
342-
// We need to continue processing after domain error handling
343-
// is complete, but not by using whatever domain was left over
344-
// when the timeout threw its exception.
345-
const domain = process.domain;
346-
process.domain = null;
347-
// If we threw, we need to process the rest of the list in nextTick.
348-
process.nextTick(listOnTimeoutNT, list);
349-
process.domain = domain;
350364
}
351365
}
352366

Collapse file

‎test/parallel/test-timers-throw-reschedule.js‎

Copy file name to clipboard
+27Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
6+
// This test checks that throwing inside a setTimeout where that Timeout
7+
// instance is the only one within its list of timeouts, doesn't cause
8+
// an issue with an unref timeout scheduled in the error handler.
9+
// Refs: https://github.com/nodejs/node/issues/19970
10+
11+
const timeout = common.platformTimeout(50);
12+
13+
const timer = setTimeout(common.mustNotCall(), 2 ** 31 - 1);
14+
15+
process.once('uncaughtException', common.mustCall((err) => {
16+
assert.strictEqual(err.message, 'setTimeout Error');
17+
18+
const now = Date.now();
19+
setTimeout(common.mustCall(() => {
20+
assert(Date.now() - now >= timeout);
21+
clearTimeout(timer);
22+
}), timeout).unref();
23+
}));
24+
25+
setTimeout(() => {
26+
throw new Error('setTimeout Error');
27+
}, timeout);

0 commit comments

Comments
0 (0)
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.