Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 632aee1

Browse filesBrowse files
shigekiMylesBorins
authored andcommitted
timers: fix not to close reused timer handle
The timer handle is reused when it is unrefed in order to avoid new callback in beforeExit(#3407). If it is unrefed within a setInterval callback, the reused timer handle is closed so that setInterval no longer keep working. This fix does not close the handle in case of setInterval. PR-URL: #11646 Reviewed-By: Julien Gilli <jgilli@nodejs.org> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 6885dcc commit 632aee1
Copy full SHA for 632aee1

File tree

Expand file treeCollapse file tree

2 files changed

+68
-1
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+68
-1
lines changed
Open diff view settings
Collapse file

‎lib/timers.js‎

Copy file name to clipboardExpand all lines: lib/timers.js
+7-1Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,6 @@ function listOnTimeout() {
222222
// As such, we can remove the list and clean up the TimerWrap C++ handle.
223223
debug('%d list empty', msecs);
224224
assert(L.isEmpty(list));
225-
this.close();
226225

227226
// Either refedLists[msecs] or unrefedLists[msecs] may have been removed and
228227
// recreated since the reference to `list` was created. Make sure they're
@@ -232,6 +231,13 @@ function listOnTimeout() {
232231
} else if (list === refedLists[msecs]) {
233232
delete refedLists[msecs];
234233
}
234+
235+
// Do not close the underlying handle if its ownership has changed
236+
// (e.g it was unrefed in its callback).
237+
if (this.owner)
238+
return;
239+
240+
this.close();
235241
}
236242

237243

Collapse file
+61Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
'use strict';
2+
// Checks that setInterval timers keep running even when they're
3+
// unrefed within their callback.
4+
5+
require('../common');
6+
const assert = require('assert');
7+
const net = require('net');
8+
9+
let counter1 = 0;
10+
let counter2 = 0;
11+
12+
// Test1 checks that clearInterval works as expected for a timer
13+
// unrefed within its callback: it removes the timer and its callback
14+
// is not called anymore. Note that the only reason why this test is
15+
// robust is that:
16+
// 1. the repeated timer it creates has a delay of 1ms
17+
// 2. when this test is completed, another test starts that creates a
18+
// new repeated timer with the same delay (1ms)
19+
// 3. because of the way timers are implemented in libuv, if two
20+
// repeated timers A and B are created in that order with the same
21+
// delay, it is guaranteed that the first occurrence of timer A
22+
// will fire before the first occurrence of timer B
23+
// 4. as a result, when the timer created by Test2 fired 11 times, if
24+
// the timer created by Test1 hadn't been removed by clearInterval,
25+
// it would have fired 11 more times, and the assertion in the
26+
// process'exit event handler would fail.
27+
function Test1() {
28+
// server only for maintaining event loop
29+
const server = net.createServer().listen(0);
30+
31+
const timer1 = setInterval(() => {
32+
timer1.unref();
33+
if (counter1++ === 3) {
34+
clearInterval(timer1);
35+
server.close(() => {
36+
Test2();
37+
});
38+
}
39+
}, 1);
40+
}
41+
42+
43+
// Test2 checks setInterval continues even if it is unrefed within
44+
// timer callback. counter2 continues to be incremented more than 11
45+
// until server close completed.
46+
function Test2() {
47+
// server only for maintaining event loop
48+
const server = net.createServer().listen(0);
49+
50+
const timer2 = setInterval(() => {
51+
timer2.unref();
52+
if (counter2++ === 3)
53+
server.close();
54+
}, 1);
55+
}
56+
57+
process.on('exit', () => {
58+
assert.strictEqual(counter1, 4);
59+
});
60+
61+
Test1();

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.