Fix segfault when a top-level-await module resumes inside AsyncLocalStorage#32184
Fix segfault when a top-level-await module resumes inside AsyncLocalStorage#32184robobun wants to merge 2 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a regression test file with two concurrent tests that spawn Bun to verify AsyncLocalStorage context is preserved across microtask/macrotask await boundaries and across top-level-await module imports, and updates the WEBKIT_VERSION preview pin used by the build scripts. ChangesAsyncLocalStorage Context Regression Tests
WebKit preview pin
Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
Updated 10:50 PM PT - Jun 16th, 2026
❌ @robobun, your commit 32d3d8f has 4 failures in
🧪 To try this PR locally: bunx bun-pr 32184That installs a local version of the PR into your bun-32184 --bun |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/regression/issue/32178.test.ts`:
- Line 13: The test uses a 1ms setTimeout to cross a macrotask boundary which
makes the test timing-sensitive; replace that real-delay timer by scheduling a
macrotask with setImmediate instead (i.e., change the Promise executor that
currently calls setTimeout with a 1ms delay to call setImmediate) so the test
crosses a macrotask boundary without an actual timed delay; update the line
containing the await new Promise(...) in issue/32178.test.ts accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c0da6beb-4663-4abf-a903-b8e16cb64a59
📒 Files selected for processing (1)
test/regression/issue/32178.test.ts
| // TEMPORARY preview pin: 6d586e293f + the asyncModuleExecutionResume fix from | ||
| // oven-sh/WebKit#252 (cherry-picked in oven-sh/WebKit#253, fixes oven-sh/bun#32178). | ||
| // Swap to an oven-sh/WebKit main autobuild SHA containing the fix before merging. | ||
| export const WEBKIT_VERSION = "autobuild-preview-pr-253-7eb8e17e"; |
There was a problem hiding this comment.
🔴 Merge precondition: WEBKIT_VERSION is currently pinned to the ephemeral preview-PR tag autobuild-preview-pr-253-7eb8e17e, which root CLAUDE.md ("Dependencies & vendoring") forbids merging. The author already flags this as TEMPORARY in the comment and PR description; this thread is the unresolved gate so it isn't merged accidentally. Once oven-sh/WebKit#252 lands on main, swap to the corresponding autobuild-<sha> from oven-sh/WebKit main and delete the TEMPORARY comment block (lines 10-12).
Extended reasoning...
What
scripts/build/deps/webkit.ts:13 sets:
export const WEBKIT_VERSION = "autobuild-preview-pr-253-7eb8e17e";This is a preview-PR autobuild tag from oven-sh/WebKit (built off an unmerged PR branch), not a main-branch autobuild SHA. The author explicitly labels it TEMPORARY in the comment block on lines 10-12 and in the PR description ("it must be swapped to the merged oven-sh/WebKit main SHA before merging this PR"), so this is not an oversight — but it is a hard merge precondition that should be tracked as an unresolved review thread rather than left to memory.
Why it's a merge blocker
Root CLAUDE.md:350 ("Dependencies & vendoring") states verbatim:
Never merge a pin to an ephemeral artifact (preview tags, unmerged-PR builds) — swap to the merged upstream SHA and verify prebuilt artifacts exist for every platform × flavor before merge.
autobuild-preview-pr-253-7eb8e17e is exactly the artifact class the rule names. Nothing in code or CI enforces the swap; only the code comment requests it. PR descriptions don't block merge — unresolved review threads do, which is why this is filed.
Concrete risks if merged as-is
- Prebuilt downloads break later.
prebuiltUrl()in this same file builds the GitHub release-asset URL directly fromWEBKIT_VERSION. Preview-PR release assets in oven-sh/WebKit are ephemeral and may be GC'd once the PR is merged/closed; anyone building this commit afterward gets a 404 on every platform × flavor. - Local-WebKit sync breaks.
scripts/sync-webkit-source.tsrunsgit checkout main && git pull && git checkout ${WEBKIT_VERSION}. A preview-PR tag isn't reachable frommain, sogit pullwon't fetch it and the subsequent checkout fails for local-WebKit developers.
Step-by-step proof
CLAUDE.md:350→ "Never merge a pin to an ephemeral artifact (preview tags, unmerged-PR builds)".webkit.ts:13→WEBKIT_VERSION = "autobuild-preview-pr-253-7eb8e17e"— a preview-PR tag, the precise thing the rule names.webkit.ts:10-12→ author comment: "TEMPORARY preview pin… Swap to an oven-sh/WebKit main autobuild SHA containing the fix before merging." Confirms intent; does not satisfy the rule, whose trigger is before merge.webkit.tsprebuiltUrl()→https://github.com/oven-sh/WebKit/releases/download/${tag}/...wheretag = WEBKIT_VERSION(it already starts withautobuild-). If the preview release is deleted after the upstream PR merges, every fresh build of this commit 404s on the prebuilt download.scripts/sync-webkit-source.ts→git checkout main; git pull; git checkout ${WEBKIT_VERSION}. A ref that exists only on a PR branch is not fetched bygit pullofmain, so the checkout fails.- Therefore the PR in its current state violates a documented hard merge rule and would leave two functional breakages on
mainif merged before the swap.
Fix
Once oven-sh/WebKit#252 is merged to oven-sh/WebKit main:
- Replace
WEBKIT_VERSIONwith the main autobuild SHA containing the fix (e.g."<40-hex-sha>"or"autobuild-<sha>"per the existing convention). - Delete the
TEMPORARYcomment block on lines 10-12. - Per
CLAUDE.md:350, verify prebuilt artifacts exist for every platform × flavor at the new tag before merging.
Then resolve this thread.
There was a problem hiding this comment.
Agreed, this is the intended merge gate. The preview pin exists only to prove the fix in CI. Before merge: oven-sh/WebKit#252 lands on WebKit main (or the one-function fix is cherry-picked onto the 6d586e lineage and published as a regular autobuild), then WEBKIT_VERSION swaps to that non-preview tag and the TEMPORARY comment block goes away. Leaving this thread open until the swap happens.
There was a problem hiding this comment.
Update after rebasing on main: main now pins WebKit 9cb85a0716 (the upstream-upgrade commit). The pin here is now autobuild-preview-pr-252-3d2a7a95, the direct preview of oven-sh/WebKit#252 which is based on 9cb85a0716, so it is ABI-compatible with main. The merge precondition is unchanged: swap to 9cb85a0716065c461bea14a0de9fe7139e5323aa (or a later main autobuild) once #252 lands on WebKit main. Leaving this open as the tracker.
There was a problem hiding this comment.
Correction to my earlier reply: the swap target is NOT 9cb85a0716 (that is #252's unfixed base, = what main pins today). When #252 lands it creates a new commit on WebKit main carrying the fix; swap WEBKIT_VERSION to that post-merge autobuild SHA. The webkit.ts comment and PR description are updated in 08b5d87.
There was a problem hiding this comment.
Second rebase update: main now pins WebKit 09f04cd5 (one more upstream upgrade past 9cb85a0716). The pin here is now autobuild-preview-pr-252-6861a738 (oven-sh/WebKit#252 rebased onto 09f04cd5). The prior preview (9cb85a0716 base) stopped compiling against main (CommonIdentifiers::globalThis drift). Merge precondition unchanged: swap to the WebKit-main autobuild SHA that includes #252 (NOT its base 09f04cd5) before merging.
There was a problem hiding this comment.
Current state after rebasing onto main (the branch was rebuilt to two clean commits): main pins WebKit cd821fec; this PR pins autobuild-preview-pr-252-83144fc3 (cd821fec + the #252 fix). Merge precondition unchanged and still tracked here: land oven-sh/WebKit#252, then swap WEBKIT_VERSION to the oven-sh/WebKit main autobuild SHA that includes it (NOT its base cd821fec). Leaving this thread open until that swap.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/build/deps/webkit.ts`:
- Around line 10-13: The WEBKIT_VERSION constant in scripts/build/deps/webkit.ts
is currently pinned to a temporary preview value
("autobuild-preview-pr-253-7eb8e17e"); replace WEBKIT_VERSION with the finalized
oven-sh/WebKit main autobuild SHA that includes the asyncModuleExecutionResume
fix so scripts that consume webkitVersion (e.g., scripts/build/config.ts and
scripts/sync-webkit-source.ts) will select the stable checkout/prebuilt; after
updating WEBKIT_VERSION, verify that matching prebuilt artifacts exist for all
target platforms/variants used by the Bun builds and update any platform/variant
mapping if necessary.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 11f905cc-2502-4538-a65c-a302ffbfa2ff
📒 Files selected for processing (2)
scripts/build/deps/webkit.tstest/regression/issue/32178.test.ts
|
CI state: the regression test passes on every lane with the pinned WebKit fix. Remaining red on build 62168 is unrelated flake (s3.test.ts, streams-leak, fetch-leak, serve-http3, each already on their 2nd retry) plus darwin-14-aarch64 test shards queuing on scarce agents; the same lanes were green for this diff on the previous build. To merge:
|
robobun
force-pushed
the
farm/4058f9bc/fix-tla-async-context-resume
branch
from
f3d0809 to
eba9e78
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/regression/issue/32178.test.ts`:
- Line 1: The regression test file at test/regression/issue/32178.test.ts
currently has only the GitHub issue URL as a comment header but is missing the
second line. Add a second comment line immediately following the issue URL
comment with a brief one-line description of the bug being tested, using the
format `// [concise description of what the bug is]` to match the established
convention for regression test headers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f922244f-c598-47ce-939b-6ff5ed5f4575
📒 Files selected for processing (2)
scripts/build/deps/webkit.tstest/regression/issue/32178.test.ts
robobun
force-pushed
the
farm/4058f9bc/fix-tla-async-context-resume
branch
from
08b5d87 to
e5ca364
Compare
|
Status for a maintainer picking this up: The diff is green. The fix is verified: regression test passes 2/2 locally on x64 debug+ASAN against the current pin ( CI red is infra, not the diff. Builds 63013 and 63026 both terminated To merge:
|
Temporary pin to autobuild-preview-pr-252-83144fc3: main's WebKit cd821fec plus the one-function asyncModuleExecutionResume fix from oven-sh/WebKit#252, rebased onto cd821fec. Swap to a non-preview oven-sh/WebKit main autobuild SHA containing the fix before merging.
robobun
force-pushed
the
farm/4058f9bc/fix-tla-async-context-resume
branch
from
d6fab7b to
32d3d8f
Compare
|
CI update (supersedes my earlier note about x64 agent capacity): the x64 build agents have recovered. Latest build on The 2 red test-bun lanes are unrelated pre-existing flaky tests, none touching this diff: Merge steps unchanged: land oven-sh/WebKit#252, then swap |
1 participant
Fixes #32178
Repro
Crashes 100% deterministically on 1.3.14 and canary (
panic(main thread): Segmentation fault at address 0x600010071/0x0/0x10), works on 1.3.13. The reporter's workload (alchemy IaC) runs its module graph inside AsyncLocalStorage, which is why every run crashed. TheBun__internal_dispatch_ready_poll/ subprocess frames in the reported stack are incidental; the crash fires with no subprocesses.Cause
Type confusion in the WebKit fork, not a GC bug. Since the 1.3.14 module-loader rewrite, async module resumption goes through
InternalMicrotask::AsyncModuleExecutionResume.JSPromise::resolveWithInternalMicrotaskForAsyncAwaitwraps the microtask context in anInternalFieldTuple(context, asyncContext)whenever Bun's async context is active at await time. TheAsyncFunctionResumedispatch unwraps that tuple;asyncModuleExecutionResumedid not, and ranuncheckedDowncast<JSModuleRecord>on the tuple (top frame of the decoded crash:JSC::runMicrotask->asyncModuleExecutionResume). In asserting builds this surfaces asJSC::reportZappedCellAndCrash; in release builds it segfaults at garbage addresses.Fix
Two parts:
AsyncModuleExecutionResumedispatch and set/restore the async context around the resumption, mirroring the existingAsyncFunctionResumecase. This also makes AsyncLocalStorage context survive across top-level await, matching Node.WEBKIT_VERSIONbump to the preview build of fish: Job 1, 'bun' terminated by signal SIGILL (Illegal instruction) #252.CI pin (temporary, must be swapped before merge)
WEBKIT_VERSIONis pinned toautobuild-preview-pr-252-83144fc3: main's current WebKit pin (cd821fec) plus the one-function fix from #252, rebased ontocd821fecso it is ABI-compatible with main's bindings. main has advanced its WebKit pin several times during review (each an upstream module-loader change); #252 is kept rebased onto whatever main currently pins, and this pin tracks it.Before merging: land oven-sh/WebKit#252, then swap
WEBKIT_VERSIONto the oven-sh/WebKit main autobuild SHA that includes it (the new commit created when it lands, not its unfixed basecd821fec). Tracked by the review thread onscripts/build/deps/webkit.ts.Verification
Built bun against the pinned preview locally (linux x64 debug+ASAN,
bun bd):test/regression/issue/32178.test.ts: 2 pass; both fail (child segfault) against the unfixed WebKittest/js/node/async_hooks/suites passThe fix still holds through the recent upstream module-loader changes (WebKit #254, bun #32437).