module-loader: don't double-fire moduleRegistryModuleSettled after inline sync replay#225
Merged
Conversation
…line sync replay hostLoadImportedModule's synchronous-replay path (JSModuleLoader.cpp branch `fetchPromise is Fulfilled`, line 712-725) calls makeModule + fetchComplete + modulePromise->fulfillPromise inline while a require(esm) drains the synchronous module queue. If a ModuleRegistryFetchSettled reaction had already run on the *normal* microtask queue for that same entry before the require(esm) entered sync mode, it left a ModuleRegistryModuleSettled reaction queued there too. When the normal queue later drained, that stale reaction re-entered fetchComplete on an entry whose status was now Fetched, tripping the "m_status == Fetching" assertion at ModuleRegistryEntry.cpp:254 (SIGABRT on Linux, arm64 PAC IB trap / SIGTRAP on macOS). moduleRegistryFetchSettled already guards this with `if (modulePromise->status() != Pending) return;` — apply the same guard symmetrically to moduleRegistryModuleSettled. Fixes oven-sh/bun#30281.
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis change adds a pending-promise guard to ChangesModule Settlement Re-entrancy Prevention
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Preview Builds
|
alii
added a commit
to oven-sh/bun
that referenced
this pull request
May 12, 2026
Jarred-Sumner
pushed a commit
to oven-sh/bun
that referenced
this pull request
May 12, 2026
…0527) ## WebKit changes (88b2f7a2 → 5488984d) Single commit on top of the previous pin: ### `module-loader: don't double-fire moduleRegistryModuleSettled after inline sync replay` (oven-sh/WebKit#225, rebased #217) **File:** `Source/JavaScriptCore/runtime/JSMicrotask.cpp` **What:** Adds a `modulePromise->status() != Pending` early-return guard to `moduleRegistryModuleSettled`, symmetric with the guard already present in `moduleRegistryFetchSettled`. Gated under `#if USE(BUN_JSC_ADDITIONS)`. **Why:** `require()` of an ESM whose graph contained a diamond dependency through a barrel deadlocked (release) / aborted on `ASSERTION FAILED: m_status == Status::Fetching` (debug). `hostLoadImportedModule`'s synchronous-replay branch (taken when `require(esm)` is draining the synchronous module queue) calls `fetchComplete` + fulfills `modulePromise` inline. If a `ModuleRegistryFetchSettled` reaction had already run on the *normal* microtask queue for the same entry before sync mode was entered, it left a stale `ModuleRegistryModuleSettled` reaction queued there. When the normal queue later drained, that reaction re-entered `fetchComplete` on an already-`Fetched` entry. No changes to `JSType.h`. No WebCore code-generator changes. --- **Verification:** - ✅ `test/regression/issue/30493.test.ts` fails on current `main` (assertion crash, empty stdout) - ✅ Same test passes on `bun run build:local` with the patched WebKit - ✅ Same test passes on `bun bd` with the prebuilt preview tarball - ✅ Full bun CI green against `autobuild-preview-pr-225-2b6b1c39` (build #53556 — 67 pass, 3 pre-existing main flakes also red on #30522) Fixes #30493 Fixes #30281 Closes #30283 (the dependency-free 6-file repro in this PR covers the same root cause without needing a react+MUI install) --------- Co-authored-by: robobun <117481402+robobun@users.noreply.github.com>
alii
added a commit
to oven-sh/bun
that referenced
this pull request
May 12, 2026
… module no longer hangs (#30537) A second `await import()` of a path whose first import threw used to hang forever instead of re-throwing. Two reported variants: - **#23139**: same path twice - **#22743**: path X, then a *different* error path Y, then X again (true regression, 1.2.20 → 1.2.21) While verifying #30527 I checked whether oven-sh/WebKit#225 covered these — turns out both were **already fixed** before that bump (main + WebKit `88b2f7a2` doesn't hang either). The fix landed somewhere in the module-loader rewrite window (#29393 → #30262); the issues just never got re-tested. | build | #23139 | #22743 | |---|---|---| | 1.3.13 | hangs | hangs | | main + WebKit `88b2f7a2` (pre-#30527) | ✅ | ✅ | | main + WebKit `5488984d` | ✅ | ✅ | Both tests fail on `USE_SYSTEM_BUN=1` (stdout missing the post-hang lines, killed by spawn timeout) and pass on `bun bd test`. Fixes #23139 Fixes #22743
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rebased cherry-pick of #217 onto current
main(so it includes #215).Fixes oven-sh/bun#30493, oven-sh/bun#30281.
Adds a
modulePromise->status() != Pendingearly-return guard tomoduleRegistryModuleSettled, symmetric with the guard already present inmoduleRegistryFetchSettled. See #217 for the full root-cause writeup.Verified locally against the oven-sh/bun#30493 repro:
bun bdat WebKit88b2f7a2(main, [JSC] innerModuleEvaluation: don't skip async-dep wait for siblings in same Evaluate() #215 only):ASSERTION FAILED: m_status == Status::Fetchingbun run build:localat this branch: prints{"a":"/","b":"barrel","shared":"/"}, exit 0Supersedes #217 (same patch, rebased).