Skip to content

Commit 7f63aa2

Browse files
committed
timers: fix the order of timers under a long loop
Fixes: #37226 (comment)
1 parent ecd385e commit 7f63aa2

File tree

4 files changed

+74
-2
lines changed

4 files changed

+74
-2
lines changed

lib/internal/priority_queue.js

+18
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,24 @@ module.exports = class PriorityQueue {
3838
return this.#heap[1];
3939
}
4040

41+
secondary() {
42+
// As the priority queue is a binary heap, the secondary element is
43+
// always the second or third element in the heap.
44+
switch (this.#size) {
45+
case 0:
46+
case 1:
47+
return undefined;
48+
49+
case 2:
50+
return this.#heap[2] === undefined ? this.#heap[3] : this.#heap[2];
51+
52+
default:
53+
return this.#compare(this.#heap[2], this.#heap[3]) < 0 ?
54+
this.#heap[2] :
55+
this.#heap[3];
56+
}
57+
}
58+
4159
percolateDown(pos) {
4260
const compare = this.#compare;
4361
const setPosition = this.#setPosition;

lib/internal/timers.js

+11-2
Original file line numberDiff line numberDiff line change
@@ -520,16 +520,25 @@ function getTimerCallbacks(runNextTicks) {
520520

521521
let ranAtLeastOneTimer = false;
522522
let timer;
523+
const secondaryList = timerListQueue.secondary();
523524
while ((timer = L.peek(list)) != null) {
524525
const diff = now - timer._idleStart;
525526

526527
// Check if this loop iteration is too early for the next timer.
527528
// This happens if there are more timers scheduled for later in the list.
528-
if (diff < msecs) {
529+
//
530+
// Else, if the secondary list is earlier than current timer, then we
531+
// return to the main loop to process the secondary list first.
532+
if (diff < msecs || secondaryList?.expiry < timer._idleStart + msecs) {
529533
list.expiry = MathMax(timer._idleStart + msecs, now + 1);
530534
list.id = timerListId++;
531535
timerListQueue.percolateDown(1);
532-
debug('%d list wait because diff is %d', msecs, diff);
536+
537+
if (diff < msecs) {
538+
debug('%d list wait because diff is %d', msecs, diff);
539+
} else {
540+
debug('%d list wait because it\'s no more earliest', msecs);
541+
}
533542
return;
534543
}
535544

test/parallel/test-priority-queue.js

+13
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,10 @@ const PriorityQueue = require('internal/priority_queue');
8585
queue.insert({ value: 3, position: null });
8686
queue.insert({ value: 4, position: null });
8787
queue.insert({ value: 5, position: null });
88+
assert.strictEqual(queue.secondary().value, 2);
8889

8990
queue.insert({ value: 2, position: null });
91+
assert.strictEqual(queue.secondary().value, 2);
9092
const secondLargest = { value: 10, position: null };
9193
queue.insert(secondLargest);
9294
const largest = { value: 15, position: null };
@@ -103,10 +105,15 @@ const PriorityQueue = require('internal/priority_queue');
103105
queue.removeAt(6);
104106

105107
assert.strictEqual(queue.shift().value, 1);
108+
assert.strictEqual(queue.secondary().value, 2);
106109
assert.strictEqual(queue.shift().value, 2);
110+
assert.strictEqual(queue.secondary().value, 4);
107111
assert.strictEqual(queue.shift().value, 2);
112+
assert.strictEqual(queue.secondary().value, 15);
108113
assert.strictEqual(queue.shift().value, 4);
114+
assert.strictEqual(queue.secondary(), undefined);
109115
assert.strictEqual(queue.shift().value, 15);
116+
assert.strictEqual(queue.secondary(), undefined);
110117

111118
assert.strictEqual(queue.shift(), undefined);
112119
}
@@ -123,11 +130,17 @@ const PriorityQueue = require('internal/priority_queue');
123130
const i8 = { value: 8, position: null };
124131

125132
queue.insert({ value: 1, position: null });
133+
assert.strictEqual(queue.secondary(), undefined);
126134
queue.insert({ value: 6, position: null });
135+
assert.strictEqual(queue.secondary().value, 6);
127136
queue.insert({ value: 2, position: null });
137+
assert.strictEqual(queue.secondary().value, 2);
128138
queue.insert(i7);
139+
assert.strictEqual(queue.secondary().value, 2);
129140
queue.insert(i8);
141+
assert.strictEqual(queue.secondary().value, 2);
130142
queue.insert(i3);
143+
assert.strictEqual(queue.secondary().value, 2);
131144

132145
assert.strictEqual(i7.position, 4);
133146
queue.removeAt(4);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
'use strict';
2+
3+
require('../common');
4+
5+
const assert = require('assert');
6+
7+
const orders = [];
8+
process.on('exit', () => {
9+
assert.deepStrictEqual(orders, [ 1, 2, 3 ]);
10+
});
11+
12+
setTimeout(() => {
13+
orders.push(1);
14+
}, 10);
15+
16+
setTimeout(() => {
17+
orders.push(2);
18+
}, 15);
19+
20+
let now = Date.now();
21+
while (Date.now() - now < 100) {
22+
//
23+
}
24+
25+
setTimeout(() => {
26+
orders.push(3);
27+
}, 10);
28+
29+
now = Date.now();
30+
while (Date.now() - now < 100) {
31+
//
32+
}

0 commit comments

Comments
 (0)