src: run same-priority platform tasks in posting order by codebytere · Pull Request #65353 · nodejs/node

codebytere · 2026-08-17T15:20:29Z

TaskQueue<T> has been a std::priority_queue since #58047 so that worker tasks honor v8::TaskPriority. Its comparator returns false for entry types without a priority member and for equal priorities, and the comment expects that to keep insertion order. A binary heap doesn't: three tasks pushed A, B, C pop as A, C, B, and 64 pushes come back out as 0, 2, 6, 14, 30, 62, ...

Three queues are affected: the per-isolate foreground queue (tasks of one priority no longer run in the order they were posted), the foreground delayed queue, and the worker runner's DelayedTaskScheduler, where it can hang shutdown. That scheduler drains its local queue in one batch, so when V8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier; the ScheduleTask then starts a timer on the scheduler's loop after all timers were stopped, and Shutdown() sits in uv_thread_join() until the delay expires (8 s when the late task is the memory reducer). It shows up as an intermittent multi-second exit stall in processes that exit soon after doing some work.

This is the other half of the shutdown race #61999 addressed: that change makes PostDelayedTask() return early once Stop() has run (both under the tasks_ lock), so every ScheduleTask that is queued was queued before the StopTask; with posting order restored it also runs before it, and no further flag is needed.

The fix gives every queued item a sequence number and uses it as the tie breaker, so equal-priority and priority-less tasks come out FIFO again while higher priorities still go first; PopAll() returns the tasks in that order instead of handing out the heap, which also removes the const_cast loops at its three call sites.

Tests: new cctest (TaskQueueTest.HigherPriorityFirstThenPostingOrder: FIFO across PopAll()/Pop() for a priority-less queue, priority-then-FIFO for TaskQueueEntry) fails on main and passes here; cctest and the default suite pass.

Refs: #58047
Refs: #61999

Disclosure: the code, test and this description were written by Claude Code, directed and reviewed by @codebytere.

codecov · 2026-08-17T19:13:16Z

Codecov Report

❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (30bff4a) to head (25b1017).
⚠️ Report is 56 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_platform.cc	94.73%	0 Missing and 1 partial ⚠️
src/node_platform.h	80.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #65353 +/- ##
==========================================
- Coverage 90.13% 90.12% -0.02% 
==========================================
Files 752 752 Lines 251568 251857 +289 Branches 47270 47364 +94 ==========================================
+ Hits 226759 226991 +232 - Misses 16168 16191 +23 - Partials 8641 8675 +34

Files with missing lines	Coverage Δ
src/node_platform.cc	`74.75% <94.73%> (-1.25%)`	⬇️
src/node_platform.h	`85.71% <80.00%> (-5.96%)`	⬇️

... and 55 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-17T23:21:17Z

CI: https://ci.nodejs.org/job/node-test-pull-request/75958/

TaskQueue became a std::priority_queue when worker tasks started to honor v8::TaskPriority. Its comparator returns false for entry types without a priority member, and for entries of equal priority, on the assumption that the heap then keeps insertion order. It does not: three tasks pushed A, B, C pop as A, C, B, and larger batches come out in heap order. That affects the per-isolate foreground task queue (tasks of one priority no longer run in the order they were posted), the foreground delayed task queue, and the delayed task scheduler of the worker thread task runner, whose local queue is drained in one batch: when v8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier, that ScheduleTask then starts a timer on the scheduler's loop after all timers were supposed to be stopped, and Shutdown() blocks in uv_thread_join() until the delay (e.g. the 8 s of the memory reducer) expires. Give every queued item a sequence number and use it as the tie breaker, so that tasks of equal priority, and tasks without one, come out in FIFO order again; higher priorities still come first. PopAll() now returns the tasks in that order instead of handing out the heap. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>

nodejs-github-bot · 2026-08-19T21:32:45Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76014/

nodejs-github-bot · 2026-08-19T22:09:35Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76015/

XinGOfCloude18 · 2026-08-19T22:29:50Z

Codecov Report
❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (30bff4a) to head (25b1017).
⚠️ Report is 51 commits behind head on main.
Files with missing lines Patch % Lines
src/node_platform.cc 94.73% 0 Missing and 1 partial ⚠️
src/node_platform.h 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #65353 +/- ##
==========================================
- Coverage 90.13% 90.12% -0.02% 
==========================================
Files 752 752 Lines 251568 251857 +289 Branches 47270 47364 +94 ==========================================
+ Hits 226759 226991 +232 - Misses 16168 16191 +23 - Partials 8641 8675 +34 
Files with missing lines Coverage Δ
src/node_platform.cc 74.75% <94.73%> (-1.25%) ⬇️
src/node_platform.h 85.71% <80.00%> (-5.96%) ⬇️
... and 55 files with indirect coverage changes
🚀 New features to boost your workflow:
❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-20T10:47:39Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76045/

nodejs-github-bot · 2026-08-20T12:16:11Z

Landed in ea4b37f

ShenHongFei · 2026-08-21T02:29:01Z

When there is a high degree of concurrency, this modification to the pr might cause node.js to freeze. For example, when a large number of https requests are initiated concurrently. After rolling back this pr, the problem disappeared. I'm not sure about the exact reason.

ShenHongFei · 2026-08-21T03:44:04Z

Reproducible script

constCONCURRENCY=64constURL='https://qq.com'asyncfunctionmain(){console.log(`🚀 Firing ${CONCURRENCY} concurrent requests → ${URL}\n`)constpromises=Array.from({length: CONCURRENCY},async(_,i)=>{conststart=Date.now()try{constres=awaitfetch(URL,{method: 'GET'})awaitres.text()console.log(`#${i+1} ✓ status=${res.status}${Date.now()-start}ms`)return{ok: true,status: res.status}}catch(err){console.log(`#${i+1} ✗ error: ${err.message}`)return{ok: false}}})constresults=awaitPromise.all(promises)constok=results.filter(r=>r.ok).lengthconsole.log(`\n✅ Done: ${ok}/${CONCURRENCY} succeeded`)}main()

ShenHongFei · 2026-08-21T03:52:11Z

TaskQueue became a std::priority_queue when worker tasks started to honor v8::TaskPriority. Its comparator returns false for entry types without a priority member, and for entries of equal priority, on the assumption that the heap then keeps insertion order. It does not: three tasks pushed A, B, C pop as A, C, B, and larger batches come out in heap order. That affects the per-isolate foreground task queue (tasks of one priority no longer run in the order they were posted), the foreground delayed task queue, and the delayed task scheduler of the worker thread task runner, whose local queue is drained in one batch: when v8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier, that ScheduleTask then starts a timer on the scheduler's loop after all timers were supposed to be stopped, and Shutdown() blocks in uv_thread_join() until the delay (e.g. the 8 s of the memory reducer) expires. Give every queued item a sequence number and use it as the tie breaker, so that tasks of equal priority, and tasks without one, come out in FIFO order again; higher priorities still come first. PopAll() now returns the tasks in that order instead of handing out the heap. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65353 Refs: #58047 Refs: #61999 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

codebytere · 2026-08-17T15:20:29Z

TaskQueue<T> has been a std::priority_queue since #58047 so that worker tasks honor v8::TaskPriority. Its comparator returns false for entry types without a priority member and for equal priorities, and the comment expects that to keep insertion order. A binary heap doesn't: three tasks pushed A, B, C pop as A, C, B, and 64 pushes come back out as 0, 2, 6, 14, 30, 62, ...

Three queues are affected: the per-isolate foreground queue (tasks of one priority no longer run in the order they were posted), the foreground delayed queue, and the worker runner's DelayedTaskScheduler, where it can hang shutdown. That scheduler drains its local queue in one batch, so when V8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier; the ScheduleTask then starts a timer on the scheduler's loop after all timers were stopped, and Shutdown() sits in uv_thread_join() until the delay expires (8 s when the late task is the memory reducer). It shows up as an intermittent multi-second exit stall in processes that exit soon after doing some work.

This is the other half of the shutdown race #61999 addressed: that change makes PostDelayedTask() return early once Stop() has run (both under the tasks_ lock), so every ScheduleTask that is queued was queued before the StopTask; with posting order restored it also runs before it, and no further flag is needed.

The fix gives every queued item a sequence number and uses it as the tie breaker, so equal-priority and priority-less tasks come out FIFO again while higher priorities still go first; PopAll() returns the tasks in that order instead of handing out the heap, which also removes the const_cast loops at its three call sites.

Tests: new cctest (TaskQueueTest.HigherPriorityFirstThenPostingOrder: FIFO across PopAll()/Pop() for a priority-less queue, priority-then-FIFO for TaskQueueEntry) fails on main and passes here; cctest and the default suite pass.

Refs: #58047
Refs: #61999

Disclosure: the code, test and this description were written by Claude Code, directed and reviewed by @codebytere.

codecov · 2026-08-17T19:13:16Z

Codecov Report

❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (30bff4a) to head (25b1017).
⚠️ Report is 56 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_platform.cc	94.73%	0 Missing and 1 partial ⚠️
src/node_platform.h	80.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #65353 +/- ##
==========================================
- Coverage 90.13% 90.12% -0.02% 
==========================================
Files 752 752 Lines 251568 251857 +289 Branches 47270 47364 +94 ==========================================
+ Hits 226759 226991 +232 - Misses 16168 16191 +23 - Partials 8641 8675 +34

Files with missing lines	Coverage Δ
src/node_platform.cc	`74.75% <94.73%> (-1.25%)`	⬇️
src/node_platform.h	`85.71% <80.00%> (-5.96%)`	⬇️

... and 55 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-17T23:21:17Z

CI: https://ci.nodejs.org/job/node-test-pull-request/75958/

TaskQueue became a std::priority_queue when worker tasks started to honor v8::TaskPriority. Its comparator returns false for entry types without a priority member, and for entries of equal priority, on the assumption that the heap then keeps insertion order. It does not: three tasks pushed A, B, C pop as A, C, B, and larger batches come out in heap order. That affects the per-isolate foreground task queue (tasks of one priority no longer run in the order they were posted), the foreground delayed task queue, and the delayed task scheduler of the worker thread task runner, whose local queue is drained in one batch: when v8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier, that ScheduleTask then starts a timer on the scheduler's loop after all timers were supposed to be stopped, and Shutdown() blocks in uv_thread_join() until the delay (e.g. the 8 s of the memory reducer) expires. Give every queued item a sequence number and use it as the tie breaker, so that tasks of equal priority, and tasks without one, come out in FIFO order again; higher priorities still come first. PopAll() now returns the tasks in that order instead of handing out the heap. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>

nodejs-github-bot · 2026-08-19T21:32:45Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76014/

nodejs-github-bot · 2026-08-19T22:09:35Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76015/

XinGOfCloude18 · 2026-08-19T22:29:50Z

Codecov Report
❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (30bff4a) to head (25b1017).
⚠️ Report is 51 commits behind head on main.
Files with missing lines Patch % Lines
src/node_platform.cc 94.73% 0 Missing and 1 partial ⚠️
src/node_platform.h 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #65353 +/- ##
==========================================
- Coverage 90.13% 90.12% -0.02% 
==========================================
Files 752 752 Lines 251568 251857 +289 Branches 47270 47364 +94 ==========================================
+ Hits 226759 226991 +232 - Misses 16168 16191 +23 - Partials 8641 8675 +34 
Files with missing lines Coverage Δ
src/node_platform.cc 74.75% <94.73%> (-1.25%) ⬇️
src/node_platform.h 85.71% <80.00%> (-5.96%) ⬇️
... and 55 files with indirect coverage changes
🚀 New features to boost your workflow:
❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-20T10:47:39Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76045/

nodejs-github-bot · 2026-08-20T12:16:11Z

Landed in ea4b37f

ShenHongFei · 2026-08-21T02:29:01Z

When there is a high degree of concurrency, this modification to the pr might cause node.js to freeze. For example, when a large number of https requests are initiated concurrently. After rolling back this pr, the problem disappeared. I'm not sure about the exact reason.

ShenHongFei · 2026-08-21T03:44:04Z

Reproducible script

constCONCURRENCY=64constURL='https://qq.com'asyncfunctionmain(){console.log(`🚀 Firing ${CONCURRENCY} concurrent requests → ${URL}\n`)constpromises=Array.from({length: CONCURRENCY},async(_,i)=>{conststart=Date.now()try{constres=awaitfetch(URL,{method: 'GET'})awaitres.text()console.log(`#${i+1} ✓ status=${res.status}${Date.now()-start}ms`)return{ok: true,status: res.status}}catch(err){console.log(`#${i+1} ✗ error: ${err.message}`)return{ok: false}}})constresults=awaitPromise.all(promises)constok=results.filter(r=>r.ok).lengthconsole.log(`\n✅ Done: ${ok}/${CONCURRENCY} succeeded`)}main()

ShenHongFei · 2026-08-21T03:52:11Z

TaskQueue became a std::priority_queue when worker tasks started to honor v8::TaskPriority. Its comparator returns false for entry types without a priority member, and for entries of equal priority, on the assumption that the heap then keeps insertion order. It does not: three tasks pushed A, B, C pop as A, C, B, and larger batches come out in heap order. That affects the per-isolate foreground task queue (tasks of one priority no longer run in the order they were posted), the foreground delayed task queue, and the delayed task scheduler of the worker thread task runner, whose local queue is drained in one batch: when v8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier, that ScheduleTask then starts a timer on the scheduler's loop after all timers were supposed to be stopped, and Shutdown() blocks in uv_thread_join() until the delay (e.g. the 8 s of the memory reducer) expires. Give every queued item a sequence number and use it as the tie breaker, so that tasks of equal priority, and tasks without one, come out in FIFO order again; higher priorities still come first. PopAll() now returns the tasks in that order instead of handing out the heap. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65353 Refs: #58047 Refs: #61999 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

codebytere · 2026-08-17T15:20:29Z

TaskQueue<T> has been a std::priority_queue since #58047 so that worker tasks honor v8::TaskPriority. Its comparator returns false for entry types without a priority member and for equal priorities, and the comment expects that to keep insertion order. A binary heap doesn't: three tasks pushed A, B, C pop as A, C, B, and 64 pushes come back out as 0, 2, 6, 14, 30, 62, ...

Three queues are affected: the per-isolate foreground queue (tasks of one priority no longer run in the order they were posted), the foreground delayed queue, and the worker runner's DelayedTaskScheduler, where it can hang shutdown. That scheduler drains its local queue in one batch, so when V8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier; the ScheduleTask then starts a timer on the scheduler's loop after all timers were stopped, and Shutdown() sits in uv_thread_join() until the delay expires (8 s when the late task is the memory reducer). It shows up as an intermittent multi-second exit stall in processes that exit soon after doing some work.

This is the other half of the shutdown race #61999 addressed: that change makes PostDelayedTask() return early once Stop() has run (both under the tasks_ lock), so every ScheduleTask that is queued was queued before the StopTask; with posting order restored it also runs before it, and no further flag is needed.

The fix gives every queued item a sequence number and uses it as the tie breaker, so equal-priority and priority-less tasks come out FIFO again while higher priorities still go first; PopAll() returns the tasks in that order instead of handing out the heap, which also removes the const_cast loops at its three call sites.

Tests: new cctest (TaskQueueTest.HigherPriorityFirstThenPostingOrder: FIFO across PopAll()/Pop() for a priority-less queue, priority-then-FIFO for TaskQueueEntry) fails on main and passes here; cctest and the default suite pass.

Refs: #58047
Refs: #61999

Disclosure: the code, test and this description were written by Claude Code, directed and reviewed by @codebytere.

codecov · 2026-08-17T19:13:16Z

Codecov Report

❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (30bff4a) to head (25b1017).
⚠️ Report is 56 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_platform.cc	94.73%	0 Missing and 1 partial ⚠️
src/node_platform.h	80.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #65353 +/- ##
==========================================
- Coverage 90.13% 90.12% -0.02% 
==========================================
Files 752 752 Lines 251568 251857 +289 Branches 47270 47364 +94 ==========================================
+ Hits 226759 226991 +232 - Misses 16168 16191 +23 - Partials 8641 8675 +34

Files with missing lines	Coverage Δ
src/node_platform.cc	`74.75% <94.73%> (-1.25%)`	⬇️
src/node_platform.h	`85.71% <80.00%> (-5.96%)`	⬇️

... and 55 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-17T23:21:17Z

CI: https://ci.nodejs.org/job/node-test-pull-request/75958/

TaskQueue became a std::priority_queue when worker tasks started to honor v8::TaskPriority. Its comparator returns false for entry types without a priority member, and for entries of equal priority, on the assumption that the heap then keeps insertion order. It does not: three tasks pushed A, B, C pop as A, C, B, and larger batches come out in heap order. That affects the per-isolate foreground task queue (tasks of one priority no longer run in the order they were posted), the foreground delayed task queue, and the delayed task scheduler of the worker thread task runner, whose local queue is drained in one batch: when v8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier, that ScheduleTask then starts a timer on the scheduler's loop after all timers were supposed to be stopped, and Shutdown() blocks in uv_thread_join() until the delay (e.g. the 8 s of the memory reducer) expires. Give every queued item a sequence number and use it as the tie breaker, so that tasks of equal priority, and tasks without one, come out in FIFO order again; higher priorities still come first. PopAll() now returns the tasks in that order instead of handing out the heap. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>

nodejs-github-bot · 2026-08-19T21:32:45Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76014/

nodejs-github-bot · 2026-08-19T22:09:35Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76015/

XinGOfCloude18 · 2026-08-19T22:29:50Z

Codecov Report
❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (30bff4a) to head (25b1017).
⚠️ Report is 51 commits behind head on main.
Files with missing lines Patch % Lines
src/node_platform.cc 94.73% 0 Missing and 1 partial ⚠️
src/node_platform.h 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #65353 +/- ##
==========================================
- Coverage 90.13% 90.12% -0.02% 
==========================================
Files 752 752 Lines 251568 251857 +289 Branches 47270 47364 +94 ==========================================
+ Hits 226759 226991 +232 - Misses 16168 16191 +23 - Partials 8641 8675 +34 
Files with missing lines Coverage Δ
src/node_platform.cc 74.75% <94.73%> (-1.25%) ⬇️
src/node_platform.h 85.71% <80.00%> (-5.96%) ⬇️
... and 55 files with indirect coverage changes
🚀 New features to boost your workflow:
❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-20T10:47:39Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76045/

nodejs-github-bot · 2026-08-20T12:16:11Z

Landed in ea4b37f

ShenHongFei · 2026-08-21T02:29:01Z

When there is a high degree of concurrency, this modification to the pr might cause node.js to freeze. For example, when a large number of https requests are initiated concurrently. After rolling back this pr, the problem disappeared. I'm not sure about the exact reason.

ShenHongFei · 2026-08-21T03:44:04Z

Reproducible script

constCONCURRENCY=64constURL='https://qq.com'asyncfunctionmain(){console.log(`🚀 Firing ${CONCURRENCY} concurrent requests → ${URL}\n`)constpromises=Array.from({length: CONCURRENCY},async(_,i)=>{conststart=Date.now()try{constres=awaitfetch(URL,{method: 'GET'})awaitres.text()console.log(`#${i+1} ✓ status=${res.status}${Date.now()-start}ms`)return{ok: true,status: res.status}}catch(err){console.log(`#${i+1} ✗ error: ${err.message}`)return{ok: false}}})constresults=awaitPromise.all(promises)constok=results.filter(r=>r.ok).lengthconsole.log(`\n✅ Done: ${ok}/${CONCURRENCY} succeeded`)}main()

ShenHongFei · 2026-08-21T03:52:11Z

TaskQueue became a std::priority_queue when worker tasks started to honor v8::TaskPriority. Its comparator returns false for entry types without a priority member, and for entries of equal priority, on the assumption that the heap then keeps insertion order. It does not: three tasks pushed A, B, C pop as A, C, B, and larger batches come out in heap order. That affects the per-isolate foreground task queue (tasks of one priority no longer run in the order they were posted), the foreground delayed task queue, and the delayed task scheduler of the worker thread task runner, whose local queue is drained in one batch: when v8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier, that ScheduleTask then starts a timer on the scheduler's loop after all timers were supposed to be stopped, and Shutdown() blocks in uv_thread_join() until the delay (e.g. the 8 s of the memory reducer) expires. Give every queued item a sequence number and use it as the tie breaker, so that tasks of equal priority, and tasks without one, come out in FIFO order again; higher priorities still come first. PopAll() now returns the tasks in that order instead of handing out the heap. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65353 Refs: #58047 Refs: #61999 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

codebytere · 2026-08-17T15:20:29Z

TaskQueue<T> has been a std::priority_queue since #58047 so that worker tasks honor v8::TaskPriority. Its comparator returns false for entry types without a priority member and for equal priorities, and the comment expects that to keep insertion order. A binary heap doesn't: three tasks pushed A, B, C pop as A, C, B, and 64 pushes come back out as 0, 2, 6, 14, 30, 62, ...

Three queues are affected: the per-isolate foreground queue (tasks of one priority no longer run in the order they were posted), the foreground delayed queue, and the worker runner's DelayedTaskScheduler, where it can hang shutdown. That scheduler drains its local queue in one batch, so when V8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier; the ScheduleTask then starts a timer on the scheduler's loop after all timers were stopped, and Shutdown() sits in uv_thread_join() until the delay expires (8 s when the late task is the memory reducer). It shows up as an intermittent multi-second exit stall in processes that exit soon after doing some work.

This is the other half of the shutdown race #61999 addressed: that change makes PostDelayedTask() return early once Stop() has run (both under the tasks_ lock), so every ScheduleTask that is queued was queued before the StopTask; with posting order restored it also runs before it, and no further flag is needed.

The fix gives every queued item a sequence number and uses it as the tie breaker, so equal-priority and priority-less tasks come out FIFO again while higher priorities still go first; PopAll() returns the tasks in that order instead of handing out the heap, which also removes the const_cast loops at its three call sites.

Tests: new cctest (TaskQueueTest.HigherPriorityFirstThenPostingOrder: FIFO across PopAll()/Pop() for a priority-less queue, priority-then-FIFO for TaskQueueEntry) fails on main and passes here; cctest and the default suite pass.

Refs: #58047
Refs: #61999

Disclosure: the code, test and this description were written by Claude Code, directed and reviewed by @codebytere.

codecov · 2026-08-17T19:13:16Z

Codecov Report

❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (30bff4a) to head (25b1017).
⚠️ Report is 56 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_platform.cc	94.73%	0 Missing and 1 partial ⚠️
src/node_platform.h	80.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #65353 +/- ##
==========================================
- Coverage 90.13% 90.12% -0.02% 
==========================================
Files 752 752 Lines 251568 251857 +289 Branches 47270 47364 +94 ==========================================
+ Hits 226759 226991 +232 - Misses 16168 16191 +23 - Partials 8641 8675 +34

Files with missing lines	Coverage Δ
src/node_platform.cc	`74.75% <94.73%> (-1.25%)`	⬇️
src/node_platform.h	`85.71% <80.00%> (-5.96%)`	⬇️

... and 55 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-17T23:21:17Z

CI: https://ci.nodejs.org/job/node-test-pull-request/75958/

TaskQueue became a std::priority_queue when worker tasks started to honor v8::TaskPriority. Its comparator returns false for entry types without a priority member, and for entries of equal priority, on the assumption that the heap then keeps insertion order. It does not: three tasks pushed A, B, C pop as A, C, B, and larger batches come out in heap order. That affects the per-isolate foreground task queue (tasks of one priority no longer run in the order they were posted), the foreground delayed task queue, and the delayed task scheduler of the worker thread task runner, whose local queue is drained in one batch: when v8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier, that ScheduleTask then starts a timer on the scheduler's loop after all timers were supposed to be stopped, and Shutdown() blocks in uv_thread_join() until the delay (e.g. the 8 s of the memory reducer) expires. Give every queued item a sequence number and use it as the tie breaker, so that tasks of equal priority, and tasks without one, come out in FIFO order again; higher priorities still come first. PopAll() now returns the tasks in that order instead of handing out the heap. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>

nodejs-github-bot · 2026-08-19T21:32:45Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76014/

nodejs-github-bot · 2026-08-19T22:09:35Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76015/

XinGOfCloude18 · 2026-08-19T22:29:50Z

Codecov Report
❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (30bff4a) to head (25b1017).
⚠️ Report is 51 commits behind head on main.
Files with missing lines Patch % Lines
src/node_platform.cc 94.73% 0 Missing and 1 partial ⚠️
src/node_platform.h 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #65353 +/- ##
==========================================
- Coverage 90.13% 90.12% -0.02% 
==========================================
Files 752 752 Lines 251568 251857 +289 Branches 47270 47364 +94 ==========================================
+ Hits 226759 226991 +232 - Misses 16168 16191 +23 - Partials 8641 8675 +34 
Files with missing lines Coverage Δ
src/node_platform.cc 74.75% <94.73%> (-1.25%) ⬇️
src/node_platform.h 85.71% <80.00%> (-5.96%) ⬇️
... and 55 files with indirect coverage changes
🚀 New features to boost your workflow:
❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-20T10:47:39Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76045/

nodejs-github-bot · 2026-08-20T12:16:11Z

Landed in ea4b37f

ShenHongFei · 2026-08-21T02:29:01Z

When there is a high degree of concurrency, this modification to the pr might cause node.js to freeze. For example, when a large number of https requests are initiated concurrently. After rolling back this pr, the problem disappeared. I'm not sure about the exact reason.

ShenHongFei · 2026-08-21T03:44:04Z

Reproducible script

constCONCURRENCY=64constURL='https://qq.com'asyncfunctionmain(){console.log(`🚀 Firing ${CONCURRENCY} concurrent requests → ${URL}\n`)constpromises=Array.from({length: CONCURRENCY},async(_,i)=>{conststart=Date.now()try{constres=awaitfetch(URL,{method: 'GET'})awaitres.text()console.log(`#${i+1} ✓ status=${res.status}${Date.now()-start}ms`)return{ok: true,status: res.status}}catch(err){console.log(`#${i+1} ✗ error: ${err.message}`)return{ok: false}}})constresults=awaitPromise.all(promises)constok=results.filter(r=>r.ok).lengthconsole.log(`\n✅ Done: ${ok}/${CONCURRENCY} succeeded`)}main()

ShenHongFei · 2026-08-21T03:52:11Z

TaskQueue became a std::priority_queue when worker tasks started to honor v8::TaskPriority. Its comparator returns false for entry types without a priority member, and for entries of equal priority, on the assumption that the heap then keeps insertion order. It does not: three tasks pushed A, B, C pop as A, C, B, and larger batches come out in heap order. That affects the per-isolate foreground task queue (tasks of one priority no longer run in the order they were posted), the foreground delayed task queue, and the delayed task scheduler of the worker thread task runner, whose local queue is drained in one batch: when v8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier, that ScheduleTask then starts a timer on the scheduler's loop after all timers were supposed to be stopped, and Shutdown() blocks in uv_thread_join() until the delay (e.g. the 8 s of the memory reducer) expires. Give every queued item a sequence number and use it as the tie breaker, so that tasks of equal priority, and tasks without one, come out in FIFO order again; higher priorities still come first. PopAll() now returns the tasks in that order instead of handing out the heap. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65353 Refs: #58047 Refs: #61999 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

codebytere · 2026-08-17T15:20:29Z

TaskQueue<T> has been a std::priority_queue since #58047 so that worker tasks honor v8::TaskPriority. Its comparator returns false for entry types without a priority member and for equal priorities, and the comment expects that to keep insertion order. A binary heap doesn't: three tasks pushed A, B, C pop as A, C, B, and 64 pushes come back out as 0, 2, 6, 14, 30, 62, ...

Three queues are affected: the per-isolate foreground queue (tasks of one priority no longer run in the order they were posted), the foreground delayed queue, and the worker runner's DelayedTaskScheduler, where it can hang shutdown. That scheduler drains its local queue in one batch, so when V8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier; the ScheduleTask then starts a timer on the scheduler's loop after all timers were stopped, and Shutdown() sits in uv_thread_join() until the delay expires (8 s when the late task is the memory reducer). It shows up as an intermittent multi-second exit stall in processes that exit soon after doing some work.

This is the other half of the shutdown race #61999 addressed: that change makes PostDelayedTask() return early once Stop() has run (both under the tasks_ lock), so every ScheduleTask that is queued was queued before the StopTask; with posting order restored it also runs before it, and no further flag is needed.

The fix gives every queued item a sequence number and uses it as the tie breaker, so equal-priority and priority-less tasks come out FIFO again while higher priorities still go first; PopAll() returns the tasks in that order instead of handing out the heap, which also removes the const_cast loops at its three call sites.

Tests: new cctest (TaskQueueTest.HigherPriorityFirstThenPostingOrder: FIFO across PopAll()/Pop() for a priority-less queue, priority-then-FIFO for TaskQueueEntry) fails on main and passes here; cctest and the default suite pass.

Refs: #58047
Refs: #61999

Disclosure: the code, test and this description were written by Claude Code, directed and reviewed by @codebytere.

codecov · 2026-08-17T19:13:16Z

Codecov Report

❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (30bff4a) to head (25b1017).
⚠️ Report is 56 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_platform.cc	94.73%	0 Missing and 1 partial ⚠️
src/node_platform.h	80.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #65353 +/- ##
==========================================
- Coverage 90.13% 90.12% -0.02% 
==========================================
Files 752 752 Lines 251568 251857 +289 Branches 47270 47364 +94 ==========================================
+ Hits 226759 226991 +232 - Misses 16168 16191 +23 - Partials 8641 8675 +34

Files with missing lines	Coverage Δ
src/node_platform.cc	`74.75% <94.73%> (-1.25%)`	⬇️
src/node_platform.h	`85.71% <80.00%> (-5.96%)`	⬇️

... and 55 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-17T23:21:17Z

CI: https://ci.nodejs.org/job/node-test-pull-request/75958/

TaskQueue became a std::priority_queue when worker tasks started to honor v8::TaskPriority. Its comparator returns false for entry types without a priority member, and for entries of equal priority, on the assumption that the heap then keeps insertion order. It does not: three tasks pushed A, B, C pop as A, C, B, and larger batches come out in heap order. That affects the per-isolate foreground task queue (tasks of one priority no longer run in the order they were posted), the foreground delayed task queue, and the delayed task scheduler of the worker thread task runner, whose local queue is drained in one batch: when v8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier, that ScheduleTask then starts a timer on the scheduler's loop after all timers were supposed to be stopped, and Shutdown() blocks in uv_thread_join() until the delay (e.g. the 8 s of the memory reducer) expires. Give every queued item a sequence number and use it as the tie breaker, so that tasks of equal priority, and tasks without one, come out in FIFO order again; higher priorities still come first. PopAll() now returns the tasks in that order instead of handing out the heap. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>

nodejs-github-bot · 2026-08-19T21:32:45Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76014/

nodejs-github-bot · 2026-08-19T22:09:35Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76015/

XinGOfCloude18 · 2026-08-19T22:29:50Z

Codecov Report
❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (30bff4a) to head (25b1017).
⚠️ Report is 51 commits behind head on main.
Files with missing lines Patch % Lines
src/node_platform.cc 94.73% 0 Missing and 1 partial ⚠️
src/node_platform.h 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #65353 +/- ##
==========================================
- Coverage 90.13% 90.12% -0.02% 
==========================================
Files 752 752 Lines 251568 251857 +289 Branches 47270 47364 +94 ==========================================
+ Hits 226759 226991 +232 - Misses 16168 16191 +23 - Partials 8641 8675 +34 
Files with missing lines Coverage Δ
src/node_platform.cc 74.75% <94.73%> (-1.25%) ⬇️
src/node_platform.h 85.71% <80.00%> (-5.96%) ⬇️
... and 55 files with indirect coverage changes
🚀 New features to boost your workflow:
❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-20T10:47:39Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76045/

nodejs-github-bot · 2026-08-20T12:16:11Z

Landed in ea4b37f

ShenHongFei · 2026-08-21T02:29:01Z

When there is a high degree of concurrency, this modification to the pr might cause node.js to freeze. For example, when a large number of https requests are initiated concurrently. After rolling back this pr, the problem disappeared. I'm not sure about the exact reason.

ShenHongFei · 2026-08-21T03:44:04Z

Reproducible script

constCONCURRENCY=64constURL='https://qq.com'asyncfunctionmain(){console.log(`🚀 Firing ${CONCURRENCY} concurrent requests → ${URL}\n`)constpromises=Array.from({length: CONCURRENCY},async(_,i)=>{conststart=Date.now()try{constres=awaitfetch(URL,{method: 'GET'})awaitres.text()console.log(`#${i+1} ✓ status=${res.status}${Date.now()-start}ms`)return{ok: true,status: res.status}}catch(err){console.log(`#${i+1} ✗ error: ${err.message}`)return{ok: false}}})constresults=awaitPromise.all(promises)constok=results.filter(r=>r.ok).lengthconsole.log(`\n✅ Done: ${ok}/${CONCURRENCY} succeeded`)}main()

ShenHongFei · 2026-08-21T03:52:11Z

TaskQueue became a std::priority_queue when worker tasks started to honor v8::TaskPriority. Its comparator returns false for entry types without a priority member, and for entries of equal priority, on the assumption that the heap then keeps insertion order. It does not: three tasks pushed A, B, C pop as A, C, B, and larger batches come out in heap order. That affects the per-isolate foreground task queue (tasks of one priority no longer run in the order they were posted), the foreground delayed task queue, and the delayed task scheduler of the worker thread task runner, whose local queue is drained in one batch: when v8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier, that ScheduleTask then starts a timer on the scheduler's loop after all timers were supposed to be stopped, and Shutdown() blocks in uv_thread_join() until the delay (e.g. the 8 s of the memory reducer) expires. Give every queued item a sequence number and use it as the tie breaker, so that tasks of equal priority, and tasks without one, come out in FIFO order again; higher priorities still come first. PopAll() now returns the tasks in that order instead of handing out the heap. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65353 Refs: #58047 Refs: #61999 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

codebytere · 2026-08-17T15:20:29Z

TaskQueue<T> has been a std::priority_queue since #58047 so that worker tasks honor v8::TaskPriority. Its comparator returns false for entry types without a priority member and for equal priorities, and the comment expects that to keep insertion order. A binary heap doesn't: three tasks pushed A, B, C pop as A, C, B, and 64 pushes come back out as 0, 2, 6, 14, 30, 62, ...

Three queues are affected: the per-isolate foreground queue (tasks of one priority no longer run in the order they were posted), the foreground delayed queue, and the worker runner's DelayedTaskScheduler, where it can hang shutdown. That scheduler drains its local queue in one batch, so when V8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier; the ScheduleTask then starts a timer on the scheduler's loop after all timers were stopped, and Shutdown() sits in uv_thread_join() until the delay expires (8 s when the late task is the memory reducer). It shows up as an intermittent multi-second exit stall in processes that exit soon after doing some work.

This is the other half of the shutdown race #61999 addressed: that change makes PostDelayedTask() return early once Stop() has run (both under the tasks_ lock), so every ScheduleTask that is queued was queued before the StopTask; with posting order restored it also runs before it, and no further flag is needed.

The fix gives every queued item a sequence number and uses it as the tie breaker, so equal-priority and priority-less tasks come out FIFO again while higher priorities still go first; PopAll() returns the tasks in that order instead of handing out the heap, which also removes the const_cast loops at its three call sites.

Tests: new cctest (TaskQueueTest.HigherPriorityFirstThenPostingOrder: FIFO across PopAll()/Pop() for a priority-less queue, priority-then-FIFO for TaskQueueEntry) fails on main and passes here; cctest and the default suite pass.

Refs: #58047
Refs: #61999

Disclosure: the code, test and this description were written by Claude Code, directed and reviewed by @codebytere.

codecov · 2026-08-17T19:13:16Z

Codecov Report

❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (30bff4a) to head (25b1017).
⚠️ Report is 56 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_platform.cc	94.73%	0 Missing and 1 partial ⚠️
src/node_platform.h	80.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #65353 +/- ##
==========================================
- Coverage 90.13% 90.12% -0.02% 
==========================================
Files 752 752 Lines 251568 251857 +289 Branches 47270 47364 +94 ==========================================
+ Hits 226759 226991 +232 - Misses 16168 16191 +23 - Partials 8641 8675 +34

Files with missing lines	Coverage Δ
src/node_platform.cc	`74.75% <94.73%> (-1.25%)`	⬇️
src/node_platform.h	`85.71% <80.00%> (-5.96%)`	⬇️

... and 55 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-17T23:21:17Z

CI: https://ci.nodejs.org/job/node-test-pull-request/75958/

TaskQueue became a std::priority_queue when worker tasks started to honor v8::TaskPriority. Its comparator returns false for entry types without a priority member, and for entries of equal priority, on the assumption that the heap then keeps insertion order. It does not: three tasks pushed A, B, C pop as A, C, B, and larger batches come out in heap order. That affects the per-isolate foreground task queue (tasks of one priority no longer run in the order they were posted), the foreground delayed task queue, and the delayed task scheduler of the worker thread task runner, whose local queue is drained in one batch: when v8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier, that ScheduleTask then starts a timer on the scheduler's loop after all timers were supposed to be stopped, and Shutdown() blocks in uv_thread_join() until the delay (e.g. the 8 s of the memory reducer) expires. Give every queued item a sequence number and use it as the tie breaker, so that tasks of equal priority, and tasks without one, come out in FIFO order again; higher priorities still come first. PopAll() now returns the tasks in that order instead of handing out the heap. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>

nodejs-github-bot · 2026-08-19T21:32:45Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76014/

nodejs-github-bot · 2026-08-19T22:09:35Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76015/

XinGOfCloude18 · 2026-08-19T22:29:50Z

Codecov Report
❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (30bff4a) to head (25b1017).
⚠️ Report is 51 commits behind head on main.
Files with missing lines Patch % Lines
src/node_platform.cc 94.73% 0 Missing and 1 partial ⚠️
src/node_platform.h 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #65353 +/- ##
==========================================
- Coverage 90.13% 90.12% -0.02% 
==========================================
Files 752 752 Lines 251568 251857 +289 Branches 47270 47364 +94 ==========================================
+ Hits 226759 226991 +232 - Misses 16168 16191 +23 - Partials 8641 8675 +34 
Files with missing lines Coverage Δ
src/node_platform.cc 74.75% <94.73%> (-1.25%) ⬇️
src/node_platform.h 85.71% <80.00%> (-5.96%) ⬇️
... and 55 files with indirect coverage changes
🚀 New features to boost your workflow:
❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-20T10:47:39Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76045/

nodejs-github-bot · 2026-08-20T12:16:11Z

Landed in ea4b37f

ShenHongFei · 2026-08-21T02:29:01Z

When there is a high degree of concurrency, this modification to the pr might cause node.js to freeze. For example, when a large number of https requests are initiated concurrently. After rolling back this pr, the problem disappeared. I'm not sure about the exact reason.

ShenHongFei · 2026-08-21T03:44:04Z

Reproducible script

constCONCURRENCY=64constURL='https://qq.com'asyncfunctionmain(){console.log(`🚀 Firing ${CONCURRENCY} concurrent requests → ${URL}\n`)constpromises=Array.from({length: CONCURRENCY},async(_,i)=>{conststart=Date.now()try{constres=awaitfetch(URL,{method: 'GET'})awaitres.text()console.log(`#${i+1} ✓ status=${res.status}${Date.now()-start}ms`)return{ok: true,status: res.status}}catch(err){console.log(`#${i+1} ✗ error: ${err.message}`)return{ok: false}}})constresults=awaitPromise.all(promises)constok=results.filter(r=>r.ok).lengthconsole.log(`\n✅ Done: ${ok}/${CONCURRENCY} succeeded`)}main()

ShenHongFei · 2026-08-21T03:52:11Z

TaskQueue became a std::priority_queue when worker tasks started to honor v8::TaskPriority. Its comparator returns false for entry types without a priority member, and for entries of equal priority, on the assumption that the heap then keeps insertion order. It does not: three tasks pushed A, B, C pop as A, C, B, and larger batches come out in heap order. That affects the per-isolate foreground task queue (tasks of one priority no longer run in the order they were posted), the foreground delayed task queue, and the delayed task scheduler of the worker thread task runner, whose local queue is drained in one batch: when v8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier, that ScheduleTask then starts a timer on the scheduler's loop after all timers were supposed to be stopped, and Shutdown() blocks in uv_thread_join() until the delay (e.g. the 8 s of the memory reducer) expires. Give every queued item a sequence number and use it as the tie breaker, so that tasks of equal priority, and tasks without one, come out in FIFO order again; higher priorities still come first. PopAll() now returns the tasks in that order instead of handing out the heap. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65353 Refs: #58047 Refs: #61999 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

codebytere · 2026-08-17T15:20:29Z

TaskQueue<T> has been a std::priority_queue since #58047 so that worker tasks honor v8::TaskPriority. Its comparator returns false for entry types without a priority member and for equal priorities, and the comment expects that to keep insertion order. A binary heap doesn't: three tasks pushed A, B, C pop as A, C, B, and 64 pushes come back out as 0, 2, 6, 14, 30, 62, ...

Three queues are affected: the per-isolate foreground queue (tasks of one priority no longer run in the order they were posted), the foreground delayed queue, and the worker runner's DelayedTaskScheduler, where it can hang shutdown. That scheduler drains its local queue in one batch, so when V8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier; the ScheduleTask then starts a timer on the scheduler's loop after all timers were stopped, and Shutdown() sits in uv_thread_join() until the delay expires (8 s when the late task is the memory reducer). It shows up as an intermittent multi-second exit stall in processes that exit soon after doing some work.

This is the other half of the shutdown race #61999 addressed: that change makes PostDelayedTask() return early once Stop() has run (both under the tasks_ lock), so every ScheduleTask that is queued was queued before the StopTask; with posting order restored it also runs before it, and no further flag is needed.

The fix gives every queued item a sequence number and uses it as the tie breaker, so equal-priority and priority-less tasks come out FIFO again while higher priorities still go first; PopAll() returns the tasks in that order instead of handing out the heap, which also removes the const_cast loops at its three call sites.

Tests: new cctest (TaskQueueTest.HigherPriorityFirstThenPostingOrder: FIFO across PopAll()/Pop() for a priority-less queue, priority-then-FIFO for TaskQueueEntry) fails on main and passes here; cctest and the default suite pass.

Refs: #58047
Refs: #61999

Disclosure: the code, test and this description were written by Claude Code, directed and reviewed by @codebytere.

codecov · 2026-08-17T19:13:16Z

Codecov Report

❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (30bff4a) to head (25b1017).
⚠️ Report is 56 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_platform.cc	94.73%	0 Missing and 1 partial ⚠️
src/node_platform.h	80.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #65353 +/- ##
==========================================
- Coverage 90.13% 90.12% -0.02% 
==========================================
Files 752 752 Lines 251568 251857 +289 Branches 47270 47364 +94 ==========================================
+ Hits 226759 226991 +232 - Misses 16168 16191 +23 - Partials 8641 8675 +34

Files with missing lines	Coverage Δ
src/node_platform.cc	`74.75% <94.73%> (-1.25%)`	⬇️
src/node_platform.h	`85.71% <80.00%> (-5.96%)`	⬇️

... and 55 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-17T23:21:17Z

CI: https://ci.nodejs.org/job/node-test-pull-request/75958/

TaskQueue became a std::priority_queue when worker tasks started to honor v8::TaskPriority. Its comparator returns false for entry types without a priority member, and for entries of equal priority, on the assumption that the heap then keeps insertion order. It does not: three tasks pushed A, B, C pop as A, C, B, and larger batches come out in heap order. That affects the per-isolate foreground task queue (tasks of one priority no longer run in the order they were posted), the foreground delayed task queue, and the delayed task scheduler of the worker thread task runner, whose local queue is drained in one batch: when v8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier, that ScheduleTask then starts a timer on the scheduler's loop after all timers were supposed to be stopped, and Shutdown() blocks in uv_thread_join() until the delay (e.g. the 8 s of the memory reducer) expires. Give every queued item a sequence number and use it as the tie breaker, so that tasks of equal priority, and tasks without one, come out in FIFO order again; higher priorities still come first. PopAll() now returns the tasks in that order instead of handing out the heap. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>

nodejs-github-bot · 2026-08-19T21:32:45Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76014/

nodejs-github-bot · 2026-08-19T22:09:35Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76015/

XinGOfCloude18 · 2026-08-19T22:29:50Z

Codecov Report
❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (30bff4a) to head (25b1017).
⚠️ Report is 51 commits behind head on main.
Files with missing lines Patch % Lines
src/node_platform.cc 94.73% 0 Missing and 1 partial ⚠️
src/node_platform.h 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #65353 +/- ##
==========================================
- Coverage 90.13% 90.12% -0.02% 
==========================================
Files 752 752 Lines 251568 251857 +289 Branches 47270 47364 +94 ==========================================
+ Hits 226759 226991 +232 - Misses 16168 16191 +23 - Partials 8641 8675 +34 
Files with missing lines Coverage Δ
src/node_platform.cc 74.75% <94.73%> (-1.25%) ⬇️
src/node_platform.h 85.71% <80.00%> (-5.96%) ⬇️
... and 55 files with indirect coverage changes
🚀 New features to boost your workflow:
❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-20T10:47:39Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76045/

nodejs-github-bot · 2026-08-20T12:16:11Z

Landed in ea4b37f

ShenHongFei · 2026-08-21T02:29:01Z

When there is a high degree of concurrency, this modification to the pr might cause node.js to freeze. For example, when a large number of https requests are initiated concurrently. After rolling back this pr, the problem disappeared. I'm not sure about the exact reason.

ShenHongFei · 2026-08-21T03:44:04Z

Reproducible script

constCONCURRENCY=64constURL='https://qq.com'asyncfunctionmain(){console.log(`🚀 Firing ${CONCURRENCY} concurrent requests → ${URL}\n`)constpromises=Array.from({length: CONCURRENCY},async(_,i)=>{conststart=Date.now()try{constres=awaitfetch(URL,{method: 'GET'})awaitres.text()console.log(`#${i+1} ✓ status=${res.status}${Date.now()-start}ms`)return{ok: true,status: res.status}}catch(err){console.log(`#${i+1} ✗ error: ${err.message}`)return{ok: false}}})constresults=awaitPromise.all(promises)constok=results.filter(r=>r.ok).lengthconsole.log(`\n✅ Done: ${ok}/${CONCURRENCY} succeeded`)}main()

ShenHongFei · 2026-08-21T03:52:11Z

TaskQueue became a std::priority_queue when worker tasks started to honor v8::TaskPriority. Its comparator returns false for entry types without a priority member, and for entries of equal priority, on the assumption that the heap then keeps insertion order. It does not: three tasks pushed A, B, C pop as A, C, B, and larger batches come out in heap order. That affects the per-isolate foreground task queue (tasks of one priority no longer run in the order they were posted), the foreground delayed task queue, and the delayed task scheduler of the worker thread task runner, whose local queue is drained in one batch: when v8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier, that ScheduleTask then starts a timer on the scheduler's loop after all timers were supposed to be stopped, and Shutdown() blocks in uv_thread_join() until the delay (e.g. the 8 s of the memory reducer) expires. Give every queued item a sequence number and use it as the tie breaker, so that tasks of equal priority, and tasks without one, come out in FIFO order again; higher priorities still come first. PopAll() now returns the tasks in that order instead of handing out the heap. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65353 Refs: #58047 Refs: #61999 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

codebytere · 2026-08-17T15:20:29Z

TaskQueue<T> has been a std::priority_queue since #58047 so that worker tasks honor v8::TaskPriority. Its comparator returns false for entry types without a priority member and for equal priorities, and the comment expects that to keep insertion order. A binary heap doesn't: three tasks pushed A, B, C pop as A, C, B, and 64 pushes come back out as 0, 2, 6, 14, 30, 62, ...

Three queues are affected: the per-isolate foreground queue (tasks of one priority no longer run in the order they were posted), the foreground delayed queue, and the worker runner's DelayedTaskScheduler, where it can hang shutdown. That scheduler drains its local queue in one batch, so when V8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier; the ScheduleTask then starts a timer on the scheduler's loop after all timers were stopped, and Shutdown() sits in uv_thread_join() until the delay expires (8 s when the late task is the memory reducer). It shows up as an intermittent multi-second exit stall in processes that exit soon after doing some work.

This is the other half of the shutdown race #61999 addressed: that change makes PostDelayedTask() return early once Stop() has run (both under the tasks_ lock), so every ScheduleTask that is queued was queued before the StopTask; with posting order restored it also runs before it, and no further flag is needed.

The fix gives every queued item a sequence number and uses it as the tie breaker, so equal-priority and priority-less tasks come out FIFO again while higher priorities still go first; PopAll() returns the tasks in that order instead of handing out the heap, which also removes the const_cast loops at its three call sites.

Tests: new cctest (TaskQueueTest.HigherPriorityFirstThenPostingOrder: FIFO across PopAll()/Pop() for a priority-less queue, priority-then-FIFO for TaskQueueEntry) fails on main and passes here; cctest and the default suite pass.

Refs: #58047
Refs: #61999

Disclosure: the code, test and this description were written by Claude Code, directed and reviewed by @codebytere.

codecov · 2026-08-17T19:13:16Z

Codecov Report

❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (30bff4a) to head (25b1017).
⚠️ Report is 56 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_platform.cc	94.73%	0 Missing and 1 partial ⚠️
src/node_platform.h	80.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #65353 +/- ##
==========================================
- Coverage 90.13% 90.12% -0.02% 
==========================================
Files 752 752 Lines 251568 251857 +289 Branches 47270 47364 +94 ==========================================
+ Hits 226759 226991 +232 - Misses 16168 16191 +23 - Partials 8641 8675 +34

Files with missing lines	Coverage Δ
src/node_platform.cc	`74.75% <94.73%> (-1.25%)`	⬇️
src/node_platform.h	`85.71% <80.00%> (-5.96%)`	⬇️

... and 55 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-17T23:21:17Z

CI: https://ci.nodejs.org/job/node-test-pull-request/75958/

TaskQueue became a std::priority_queue when worker tasks started to honor v8::TaskPriority. Its comparator returns false for entry types without a priority member, and for entries of equal priority, on the assumption that the heap then keeps insertion order. It does not: three tasks pushed A, B, C pop as A, C, B, and larger batches come out in heap order. That affects the per-isolate foreground task queue (tasks of one priority no longer run in the order they were posted), the foreground delayed task queue, and the delayed task scheduler of the worker thread task runner, whose local queue is drained in one batch: when v8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier, that ScheduleTask then starts a timer on the scheduler's loop after all timers were supposed to be stopped, and Shutdown() blocks in uv_thread_join() until the delay (e.g. the 8 s of the memory reducer) expires. Give every queued item a sequence number and use it as the tie breaker, so that tasks of equal priority, and tasks without one, come out in FIFO order again; higher priorities still come first. PopAll() now returns the tasks in that order instead of handing out the heap. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>

nodejs-github-bot · 2026-08-19T21:32:45Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76014/

nodejs-github-bot · 2026-08-19T22:09:35Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76015/

XinGOfCloude18 · 2026-08-19T22:29:50Z

Codecov Report
❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (30bff4a) to head (25b1017).
⚠️ Report is 51 commits behind head on main.
Files with missing lines Patch % Lines
src/node_platform.cc 94.73% 0 Missing and 1 partial ⚠️
src/node_platform.h 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #65353 +/- ##
==========================================
- Coverage 90.13% 90.12% -0.02% 
==========================================
Files 752 752 Lines 251568 251857 +289 Branches 47270 47364 +94 ==========================================
+ Hits 226759 226991 +232 - Misses 16168 16191 +23 - Partials 8641 8675 +34 
Files with missing lines Coverage Δ
src/node_platform.cc 74.75% <94.73%> (-1.25%) ⬇️
src/node_platform.h 85.71% <80.00%> (-5.96%) ⬇️
... and 55 files with indirect coverage changes
🚀 New features to boost your workflow:
❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-20T10:47:39Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76045/

nodejs-github-bot · 2026-08-20T12:16:11Z

Landed in ea4b37f

ShenHongFei · 2026-08-21T02:29:01Z

When there is a high degree of concurrency, this modification to the pr might cause node.js to freeze. For example, when a large number of https requests are initiated concurrently. After rolling back this pr, the problem disappeared. I'm not sure about the exact reason.

ShenHongFei · 2026-08-21T03:44:04Z

Reproducible script

constCONCURRENCY=64constURL='https://qq.com'asyncfunctionmain(){console.log(`🚀 Firing ${CONCURRENCY} concurrent requests → ${URL}\n`)constpromises=Array.from({length: CONCURRENCY},async(_,i)=>{conststart=Date.now()try{constres=awaitfetch(URL,{method: 'GET'})awaitres.text()console.log(`#${i+1} ✓ status=${res.status}${Date.now()-start}ms`)return{ok: true,status: res.status}}catch(err){console.log(`#${i+1} ✗ error: ${err.message}`)return{ok: false}}})constresults=awaitPromise.all(promises)constok=results.filter(r=>r.ok).lengthconsole.log(`\n✅ Done: ${ok}/${CONCURRENCY} succeeded`)}main()

ShenHongFei · 2026-08-21T03:52:11Z

TaskQueue became a std::priority_queue when worker tasks started to honor v8::TaskPriority. Its comparator returns false for entry types without a priority member, and for entries of equal priority, on the assumption that the heap then keeps insertion order. It does not: three tasks pushed A, B, C pop as A, C, B, and larger batches come out in heap order. That affects the per-isolate foreground task queue (tasks of one priority no longer run in the order they were posted), the foreground delayed task queue, and the delayed task scheduler of the worker thread task runner, whose local queue is drained in one batch: when v8 posts a delayed worker task shortly before the platform shuts down, the StopTask pushed by Stop() can run before a ScheduleTask that was pushed earlier, that ScheduleTask then starts a timer on the scheduler's loop after all timers were supposed to be stopped, and Shutdown() blocks in uv_thread_join() until the delay (e.g. the 8 s of the memory reducer) expires. Give every queued item a sequence number and use it as the tie breaker, so that tasks of equal priority, and tasks without one, come out in FIFO order again; higher priorities still come first. PopAll() now returns the tasks in that order instead of handing out the heap. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65353 Refs: #58047 Refs: #61999 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

codebytere requested review from jasnell, joyeecheung and santigimeno August 17, 2026 15:21

codebytere added request-ci Add this label to start a Jenkins CI on a PR. c++ Issues and PRs that require attention from people who are familiar with C++. labels Aug 17, 2026

codebytere mentioned this pull request Aug 17, 2026
perf: boot every Node.js-hosting process from the Node snapshot / code cache electron/electron#52873
Merged
3 tasks

jasnell reviewed Aug 17, 2026
View reviewed changes

Comment threadsrc/node_platform.cc Outdated

jasnell approved these changes Aug 17, 2026
View reviewed changes

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 17, 2026

codebytere force-pushed the fix/platform-task-queue-fifo branch from 121482b to 25b1017Compare August 18, 2026 08:12

codebytere added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Aug 18, 2026

codebytere requested review from aduh95 and anonrig and removed request for joyeecheung and santigimeno August 19, 2026 08:53

anonrig approved these changes Aug 19, 2026
View reviewed changes

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 19, 2026

codebytere added the commit-queue PRs queued for automated landing through the Commit Queue. label Aug 20, 2026

nodejs-github-bot merged commit ea4b37f into nodejs:mainAug 20, 2026
74 checks passed

nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Aug 20, 2026

github-actionsBot mentioned this pull request Aug 25, 2026
2026-08-26, Version 26.8.0 (Current) #65551
Merged