Skip to content

[JSC] The microtask queue aborts the process at 2^25 pending tasks - #674

Open
robobun wants to merge 2 commits into
mainfrom
robobun/5f417ff6/microtask-queue-capacity
Open

robobun wants to merge 2 commits into
mainfrom
robobun/5f417ff6/microtask-queue-capacity

Conversation

@robobun

@robobun robobun commented Sep 16, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • The process aborts at 2^25 pending microtasks: panic(main thread): abort() called, exit 134, also inside try/catch. 2^25 queueMicrotask(f) calls do it in Bun, and 2^25 p.then(f) calls do it in jsc. Node accepts both. A fuzzing run found it, no user reported it, and it is not a regression. This is the microtask queue section of Script-sized containers that still abort the process when they cannot grow: the microtask queue and the web streams request queues bun#42648.
  • MarkedMicrotaskDeque keeps its tasks in a WTF::Deque, which doubles one Vector buffer. isValidCapacityForVector (wtf/Vector.h:212) limits a buffer to 2^31 - 1 bytes, so the step to a capacity of 2^26 tasks calls CRASH() in VectorBufferBase::allocateBuffer().

Fix

  • Specialize isValidCapacityForVector for QueuedTask. The limit becomes what VectorBufferBase::m_capacity (31 bits) can hold, a capacity of 2^30 tasks. The CRASH() is still there at that size, but the step to it needs 64 GB for the queue alone.
  • Correct because nothing else depends on the byte limit: Deque keeps its indices in size_t, allocateBuffer() computes the byte size in size_t, and VectorTypeOperations::move copies by pointer difference.
  • enqueue(), dequeue() and visitAggregate() do not change, so the hot path is the same code.
  • Verified: new JSTests/stress/microtask-queue-more-than-2-25-tasks.js exits 134 before and passes after (release and debug ASAN). No CI leg has the memory for it, so a static_assert at the Deque member fails the build if the specialization is lost. 3,638 promise and microtask stress test runs pass.

Background

  • QueuedTask is the record of one microtask: 40 bytes in Bun, 32 upstream.
  • A throwing enqueue() is not an option. 33 call sites are in JSPromise.cpp, in void paths (settle, await) that cannot throw or undo a half-queued settle.
  • The specialization comes before MarkedMicrotaskDeque, the first use of Deque<QueuedTask>. Only this header defines QueuedTask, so every user sees it.
Notes

Repros. Bun 1.4.3 canary (09bb54630, linux x64): exit 134 after 1.6 s. Node v26.3.0 queues and drains the same count.

const f = () => {};
for (let i = 0; i < 33554431; i++) queueMicrotask(f);

jsc (this is the new test):

const p = Promise.resolve();
const f = () => {};
for (let i = 0; i < 2 ** 25 + 1; i++) p.then(f);
drainMicrotasks();

The same abort is reachable where one call queues many tasks: resolve() on a promise with 2^25 reactions, Promise.all or Promise.race over an array of that length, and a WritableStream with 2^25 pending writer.write() promises that errors.

The test. It needs about 4.5 GB in jsc (the queue buffer, the old buffer while the Deque copies, and 2^25 result promises), so it has //@ skip if $memoryLimited, which CI passes. Locally, release build: base exits 134 after 3.6 s, this branch passes in 4.4 s with a peak RSS of 4.4 GB. Debug ASAN build: passes in 281 s, peak RSS 4.8 GB, no report.

What this does not change. The Deque keeps its costs: growth holds the old and the new buffer at once, and the buffer is never given back. After two bursts of 16 million tasks the two deques of a MicrotaskQueue keep 640 MB each.

The alternative I built first is #667 (closed): a queue of 16 KB segments, which has no single large allocation and frees memory as it drains. Its measurements are there: timing within noise on 13 patterns, one burst of 16M tasks 33% faster, peak RSS over three such bursts 1.4 GB against 2.7 GB. It replaces the container on the path of every await and it conflicts with the prepend() and takeLast() that #427 adds, so it is not the right size for this abort. It can come back as its own change if the memory behaviour matters.

Bun PR that pins the preview build of this branch and adds the Bun test: oven-sh/bun#42903.

MarkedMicrotaskDeque keeps its tasks in a WTF::Deque<QueuedTask>. A Deque doubles one Vector buffer, and
isValidCapacityForVector() limits a Vector buffer to 2^31 - 1 bytes, so the step from a capacity of 2^25 tasks to
2^26 is not valid and VectorBufferBase::allocateBuffer() calls CRASH(). Script reaches that count with 1.3 GB of
queue: 2^25 reactions on one promise, or 2^25 queueMicrotask() calls in Bun. MicrotaskQueue::enqueue() has no
failure channel, and most of its callers (settling a promise, await) cannot throw.

Specialize isValidCapacityForVector() for QueuedTask so that the only limit is what VectorBufferBase::m_capacity
(31 bits) can hold. Deque keeps its indices in size_t and allocateBuffer() computes the byte size in size_t, so
nothing else depends on the byte limit. enqueue(), dequeue() and visitAggregate() do not change.

* JSTests/stress/microtask-queue-more-than-2-25-tasks.js: Added. Aborts without the change. It needs about 4.5 GB,
  so it is skipped when memory is limited.
* Source/JavaScriptCore/runtime/MicrotaskQueue.h:
@coderabbitai

coderabbitai Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 9a7d4876-1ed3-443c-bfb8-26c3575ae349

📥 Commits

Reviewing files that changed from the base of the PR and between c34a182 and 6bf5bef.

📒 Files selected for processing (1)
  • Source/JavaScriptCore/runtime/MicrotaskQueue.h

Included review availability: Your plan provides up to 5 included reviews per hour; 0 remain after this review.


Walkthrough

The change adds bounded capacity validation, a compile-time check for large queued-task capacity, and a stress test that verifies execution and FIFO ordering after the queue exceeds 2^25 tasks.

Changes

Microtask queue capacity

Layer / File(s) Summary
Queued task capacity validation
Source/JavaScriptCore/runtime/MicrotaskQueue.h
Adds conditional capacity validation for JSC::QueuedTask and checks that capacity 1 << 26 is accepted.
Large queue execution stress test
JSTests/stress/microtask-queue-more-than-2-25-tasks.js
Adds a memory-limited test that queues more than 2^25 promise jobs, verifies that all jobs run, and checks FIFO ordering for five subsequent jobs.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 6bf5b

No actionable merge-blocking risk is identified for this change.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the primary change: the JavaScriptCore microtask queue aborts when approximately 2^25 tasks are pending.
Description check ✅ Passed The description is detailed and directly explains the problem, fix, implementation rationale, testing, memory requirements, and scope. It does not include the Bugzilla URL, Reviewed by line, or a temp…

Warning

Git: CodeRabbit could not clone the repository, so clone-backed analysis was skipped and this review may be incomplete. Verify repository clone access, such as SSH credentials, before requesting another full review. If clone access is intentionally unavailable, use path_filters to narrow the review scope.


Comment @coderabbitai help to get the list of available commands.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked the ODR question for the specialization: QueuedTask is complete only in MicrotaskQueue.h, and both Deque<QueuedTask> instantiations (MicrotaskQueue.h:232 and MicrotaskQueueInlines.h:82) sit after the specialization in this header, so no TU sees the generic predicate. The byte arithmetic past 2^31 is also fine: allocateBuffer/reallocateBuffer compute sizeToAllocate in size_t and store the element count in the 31-bit m_capacity, which the new bound respects.

Extended reasoning...

Two inline findings are already posted (the regression test is skipped on every CI leg, and the queue still aborts at the 2^30 to 2^31 doubling). This note records only what was examined and ruled out beyond them: the ODR/visibility risk of the explicit specialization, which the grep across Source/ shows is safe because every Deque<QueuedTask> instantiation goes through this header after the specialization, and the Vector/Deque size arithmetic, which uses size_t for byte sizes and keeps m_capacity as an element count that the new bound (min of 2^31-1 and SIZE_MAX/sizeof) fits in. The change is small, but the inline findings are substantive enough that a human should weigh the test's effective coverage and the remaining hard ceiling before merging.

Comment thread JSTests/stress/microtask-queue-more-than-2-25-tasks.js
Comment thread Source/JavaScriptCore/runtime/MicrotaskQueue.h
@github-actions

github-actions Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Preview build of 6bf5bef: autobuild-preview-pr-674-6bf5befa

…zation is missing

The test for the 2^25 task limit needs 4.5 GB, and CI runs the stress tests with a 4 GB limit, so no CI
configuration runs it. A static_assert next to the Deque member fails the build when the specialization of
isValidCapacityForVector for QueuedTask is dropped, or when it moves below MarkedMicrotaskDeque.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The follow-up commit (6bf5bef) adds the compile-time guard, and this second automated pass found no bugs in the updated diff; since Source/JavaScriptCore/runtime/MicrotaskQueue.h is a core engine header under CODEOWNERS, a human look from a JSC reviewer is still worthwhile before merging.

What was reviewed:

  • The specialization's mechanics: Deque keeps m_start/m_end/m_capacityMask as size_t and allocateBuffer() computes sizeToAllocate in size_t, so the new bound (min(UINT_MAX >> 1, SIZE_MAX / sizeof(QueuedTask))) fits the 31-bit m_capacity field with no narrowing on the path.
  • The second Deque<QueuedTask> instantiation site, MicrotaskQueueInlines.h:82 (remaining), is reached only via this header, so it sees the specialization; the static_assert in MarkedMicrotaskDeque would catch a future reorder.
  • The 1 << 26 argument to the static_assert is an int literal converting to size_t, which is fine.
Extended reasoning...

Overview

The PR adds an explicit specialization of WTF::isValidCapacityForVector<JSC::QueuedTask> in /home/claude/webkit/Source/JavaScriptCore/runtime/MicrotaskQueue.h (gated by USE(BUN_JSC_ADDITIONS)), lifting the 2 GB byte cap on the microtask deque's backing Vector buffer to the element-count limit of the 31-bit m_capacity field, plus a static_assert inside MarkedMicrotaskDeque that fails the build if the specialization is missing or placed after the first instantiation. It also adds a new stress test /home/claude/webkit/JSTests/stress/microtask-queue-more-than-2-25-tasks.js, marked skip if $memoryLimited and slow!, which cannot run on any CI lane. The second commit responds to the earlier inline comment asking for a compile-time guard.

Security risks

None specific to this change. The specialization only widens a capacity check; the allocation size is still computed in size_t by VectorBufferBase::allocateBuffer (/home/claude/webkit/Source/WTF/wtf/Vector.h:233), the bound explicitly caps at SIZE_MAX / sizeof(QueuedTask) so the multiplication cannot overflow, and the resulting capacity still fits the 31-bit m_capacity bitfield. A runaway producer can still exhaust memory, but that is the pre-existing behavior at a higher threshold and was already noted inline on the previous version.

Level of scrutiny

Moderate. The diff is about 25 lines and self-contained, but it modifies a core engine header (/Source/JavaScriptCore is listed under @ WebKit/jsc-reviewers in .github/CODEOWNERS, inherited from upstream), it depends on a C++ subtlety (explicit specialization must precede implicit instantiation), and the only runtime regression test is skipped everywhere in CI. The author also chose this approach over a segmented-queue alternative (#667); that design trade-off is one a maintainer should endorse. For these reasons approval is withheld even though no defects were found.

Other factors

The bug hunt exited on dry_streak with no findings on this version. I checked both Deque<QueuedTask> instantiation sites (MicrotaskQueue.h:236 and MicrotaskQueueInlines.h:82) and confirmed the latter is only reachable through this header. The new test follows upstream's shouldBe/throw conventions and lives in JSTests/stress, but violates the README's 200 ms guideline by design; it is a new fork-only file rather than a modified upstream test, so an entry in JSTests/BUN-TEST-DIFFERENCES.md is arguably not required, though a maintainer may want one since the file carries non-default run directives. The coderabbitai comment content is withheld, so I cannot confirm whether it raised objections; that is another reason to leave the final call to a human.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant