From 7f63aa2dbb5eeb832567e8d308da4003b781e88f Mon Sep 17 00:00:00 2001
From: XadillaX <i@2333.moe>
Date: Tue, 14 Feb 2023 14:04:34 +0800
Subject: [PATCH] timers: fix the order of timers under a long loop

Fixes: https://github.com/nodejs/node/issues/37226#issuecomment-774391348
---
 lib/internal/priority_queue.js                | 18 +++++++++++
 lib/internal/timers.js                        | 13 ++++++--
 test/parallel/test-priority-queue.js          | 13 ++++++++
 .../test-timers-timeout-with-long-loop.js     | 32 +++++++++++++++++++
 4 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100644 test/parallel/test-timers-timeout-with-long-loop.js

diff --git a/lib/internal/priority_queue.js b/lib/internal/priority_queue.js
index 22f89a90ef07f1..0f4a16f5071058 100644
--- a/lib/internal/priority_queue.js
+++ b/lib/internal/priority_queue.js
@@ -38,6 +38,24 @@ module.exports = class PriorityQueue {
     return this.#heap[1];
   }
 
+  secondary() {
+    // As the priority queue is a binary heap, the secondary element is
+    // always the second or third element in the heap.
+    switch (this.#size) {
+      case 0:
+      case 1:
+        return undefined;
+
+      case 2:
+        return this.#heap[2] === undefined ? this.#heap[3] : this.#heap[2];
+
+      default:
+        return this.#compare(this.#heap[2], this.#heap[3]) < 0 ?
+          this.#heap[2] :
+          this.#heap[3];
+    }
+  }
+
   percolateDown(pos) {
     const compare = this.#compare;
     const setPosition = this.#setPosition;
diff --git a/lib/internal/timers.js b/lib/internal/timers.js
index dac5938eabd7df..b25d516a8b3db0 100644
--- a/lib/internal/timers.js
+++ b/lib/internal/timers.js
@@ -520,16 +520,25 @@ function getTimerCallbacks(runNextTicks) {
 
     let ranAtLeastOneTimer = false;
     let timer;
+    const secondaryList = timerListQueue.secondary();
     while ((timer = L.peek(list)) != null) {
       const diff = now - timer._idleStart;
 
       // Check if this loop iteration is too early for the next timer.
       // This happens if there are more timers scheduled for later in the list.
-      if (diff < msecs) {
+      //
+      // Else, if the secondary list is earlier than current timer, then we
+      // return to the main loop to process the secondary list first.
+      if (diff < msecs || secondaryList?.expiry < timer._idleStart + msecs) {
         list.expiry = MathMax(timer._idleStart + msecs, now + 1);
         list.id = timerListId++;
         timerListQueue.percolateDown(1);
-        debug('%d list wait because diff is %d', msecs, diff);
+
+        if (diff < msecs) {
+          debug('%d list wait because diff is %d', msecs, diff);
+        } else {
+          debug('%d list wait because it\'s no more earliest', msecs);
+        }
         return;
       }
 
diff --git a/test/parallel/test-priority-queue.js b/test/parallel/test-priority-queue.js
index b98d228b08763b..5472233371c7c8 100644
--- a/test/parallel/test-priority-queue.js
+++ b/test/parallel/test-priority-queue.js
@@ -85,8 +85,10 @@ const PriorityQueue = require('internal/priority_queue');
   queue.insert({ value: 3, position: null });
   queue.insert({ value: 4, position: null });
   queue.insert({ value: 5, position: null });
+  assert.strictEqual(queue.secondary().value, 2);
 
   queue.insert({ value: 2, position: null });
+  assert.strictEqual(queue.secondary().value, 2);
   const secondLargest = { value: 10, position: null };
   queue.insert(secondLargest);
   const largest = { value: 15, position: null };
@@ -103,10 +105,15 @@ const PriorityQueue = require('internal/priority_queue');
   queue.removeAt(6);
 
   assert.strictEqual(queue.shift().value, 1);
+  assert.strictEqual(queue.secondary().value, 2);
   assert.strictEqual(queue.shift().value, 2);
+  assert.strictEqual(queue.secondary().value, 4);
   assert.strictEqual(queue.shift().value, 2);
+  assert.strictEqual(queue.secondary().value, 15);
   assert.strictEqual(queue.shift().value, 4);
+  assert.strictEqual(queue.secondary(), undefined);
   assert.strictEqual(queue.shift().value, 15);
+  assert.strictEqual(queue.secondary(), undefined);
 
   assert.strictEqual(queue.shift(), undefined);
 }
@@ -123,11 +130,17 @@ const PriorityQueue = require('internal/priority_queue');
   const i8 = { value: 8, position: null };
 
   queue.insert({ value: 1, position: null });
+  assert.strictEqual(queue.secondary(), undefined);
   queue.insert({ value: 6, position: null });
+  assert.strictEqual(queue.secondary().value, 6);
   queue.insert({ value: 2, position: null });
+  assert.strictEqual(queue.secondary().value, 2);
   queue.insert(i7);
+  assert.strictEqual(queue.secondary().value, 2);
   queue.insert(i8);
+  assert.strictEqual(queue.secondary().value, 2);
   queue.insert(i3);
+  assert.strictEqual(queue.secondary().value, 2);
 
   assert.strictEqual(i7.position, 4);
   queue.removeAt(4);
diff --git a/test/parallel/test-timers-timeout-with-long-loop.js b/test/parallel/test-timers-timeout-with-long-loop.js
new file mode 100644
index 00000000000000..f78f3eeae65c45
--- /dev/null
+++ b/test/parallel/test-timers-timeout-with-long-loop.js
@@ -0,0 +1,32 @@
+'use strict';
+
+require('../common');
+
+const assert = require('assert');
+
+const orders = [];
+process.on('exit', () => {
+  assert.deepStrictEqual(orders, [ 1, 2, 3 ]);
+});
+
+setTimeout(() => {
+  orders.push(1);
+}, 10);
+
+setTimeout(() => {
+  orders.push(2);
+}, 15);
+
+let now = Date.now();
+while (Date.now() - now < 100) {
+  //
+}
+
+setTimeout(() => {
+  orders.push(3);
+}, 10);
+
+now = Date.now();
+while (Date.now() - now < 100) {
+  //
+}