From 56b8426b72f56dee62e22fb268dac5699086aac4 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 20 Aug 2026 03:13:15 +0000 Subject: [PATCH] JSModuleLoader: a top-level load stores its failure into the entry it loaded moduleLoadTopRejected and moduleLoadStoreError look the entry up by key when a top-level load (import(), require(esm), the entry point) fails. The host can remove that entry while the load is in flight (removeEntry(): delete require.cache[], mock.module(), plugin modules), and a replacement load of the key can register a new entry before the removed load fails. The key lookup then stores the removed load's error into the replacement's entry, and every later import() of the key rejects with it although the replacement loaded fine. A load whose fetch was rejected could likewise store its rejection into an entry that a concurrent load of the key had registered in the meantime. The two top-level ModuleLoadingContexts now record the entry their load attaches to: moduleLoadTopSettled records the entry provideFetch() registered, and the second loadModule overload records the entry it links and evaluates. The two error sites store into that entry, the way the per-entry steps (moduleLoadStep, moduleRegistryFetchSettled) already store into the entry they hold. While the entry is registered this is the entry the key lookup found, so nothing changes. Once it is removed, the error stays with the detached entry. A load that failed at its fetch has no entry and stores nothing; moduleLoadTopSettled registers nothing for such a load since 319474@main. Under USE(BUN_JSC_ADDITIONS): only removeEntry() and clearAll() can make the key's entry differ from the load's. --- Source/JavaScriptCore/runtime/JSMicrotask.cpp | 20 ++++++++++++++++++- .../JavaScriptCore/runtime/JSModuleLoader.cpp | 12 +++++++++++ .../JavaScriptCore/runtime/JSModuleLoader.h | 6 ++++-- .../runtime/ModuleLoadingContext.cpp | 7 +++++++ .../runtime/ModuleLoadingContext.h | 11 ++++++++++ 5 files changed, 53 insertions(+), 3 deletions(-) diff --git a/Source/JavaScriptCore/runtime/JSMicrotask.cpp b/Source/JavaScriptCore/runtime/JSMicrotask.cpp index 7bf56f7caa3a..21b513fe04c6 100644 --- a/Source/JavaScriptCore/runtime/JSMicrotask.cpp +++ b/Source/JavaScriptCore/runtime/JSMicrotask.cpp @@ -1177,6 +1177,10 @@ static void moduleLoadTopSettled(JSGlobalObject* globalObject, VM& vm, ThrowScop } #if USE(BUN_JSC_ADDITIONS) } + // The entry the key holds now, registered by provideFetch() just above or + // by provideModule() before the load, is the one the loadModule below + // loads; moduleLoadTopRejected stores into it. + context->setEntry(vm, globalObject->moduleLoader()->ensureRegistered(globalObject, specifier, type)); #endif JSPromise* statePromise = JSPromise::create(vm, globalObject->promiseStructure()); @@ -1259,11 +1263,19 @@ static void moduleLoadTopRejected(JSGlobalObject* globalObject, VM& vm, std::spa if (status == JSPromise::Status::Fulfilled) resultPromise->fulfill(vm, arguments[1]); else { +#if USE(BUN_JSC_ADDITIONS) + // The entry this load loaded (ModuleLoadingContext::setEntry). Null when + // the load failed at its fetch: moduleLoadTopSettled registers nothing for + // that, or has stored the error itself, so there is nothing left to store. + ModuleRegistryEntry* entry = context->entry(); +#else const Identifier& specifier = context->moduleRequest().m_specifier; auto type = context->moduleRequest().type(); // https://html.spec.whatwg.org/multipage/webappapis.html#fetch-a-single-module-script step 13.1 // Only set an error if the entry already exists. - if (ModuleRegistryEntry* entry = globalObject->moduleLoader()->getRegisteredMayBeNull(specifier, type)) + ModuleRegistryEntry* entry = globalObject->moduleLoader()->getRegisteredMayBeNull(specifier, type); +#endif + if (entry) entry->setEvaluationError(globalObject, arguments[1]); resultPromise->reject(vm, arguments[1]); } @@ -1416,9 +1428,15 @@ static void moduleLoadStoreError(JSGlobalObject* globalObject, std::spanmoduleRequest().m_specifier; auto type = context->moduleRequest().type(); +#if USE(BUN_JSC_ADDITIONS) + // The entry this load loaded (ModuleLoadingContext::setEntry), recorded by + // the loadModule overload that created this context. + ModuleRegistryEntry* entry = context->entry(); +#else // https://html.spec.whatwg.org/multipage/webappapis.html#fetch-a-single-module-script step 13.1 // Only set an error if the entry already exists. ModuleRegistryEntry* entry = globalObject->moduleLoader()->getRegisteredMayBeNull(specifier, type); +#endif if (!entry) return; if (auto* error = dynamicDowncast(errorValue)) { diff --git a/Source/JavaScriptCore/runtime/JSModuleLoader.cpp b/Source/JavaScriptCore/runtime/JSModuleLoader.cpp index 94aa4729a482..5c5698d752f7 100644 --- a/Source/JavaScriptCore/runtime/JSModuleLoader.cpp +++ b/Source/JavaScriptCore/runtime/JSModuleLoader.cpp @@ -844,10 +844,22 @@ JSPromise* JSModuleLoader::loadModule(JSGlobalObject* globalObject, const Module VM& vm = globalObject->vm(); auto scope = DECLARE_THROW_SCOPE(vm); +#if USE(BUN_JSC_ADDITIONS) + // Every caller has just registered the key (provideFetch()). That entry is the + // one this load links and evaluates, and the one moduleLoadStoreError stores + // its failure into. Read before hostLoadImportedModule(), whose resolve() hook + // runs host code, so that it is the entry moduleLoadTopSettled recorded too. + ModuleRegistryEntry* entry = getRegisteredMayBeNull(moduleRequest.m_specifier, moduleRequest.type()); +#endif + JSPromise* promise = hostLoadImportedModule(globalObject, referrer, moduleRequest, payload, scriptFetcher, flags.contains(ModuleLoadFlag::UseImportMap)); RETURN_IF_EXCEPTION(scope, nullptr); auto* context = ModuleLoadingContext::create(vm, moduleRequest, WTF::move(scriptFetcher), flags); +#if USE(BUN_JSC_ADDITIONS) + if (entry) + context->setEntry(vm, entry); +#endif JSPromise* resultPromise = JSPromise::create(vm, globalObject->promiseStructure()); resultPromise->markAsHandled(); diff --git a/Source/JavaScriptCore/runtime/JSModuleLoader.h b/Source/JavaScriptCore/runtime/JSModuleLoader.h index bb3aa3778c91..d0ac4f701a54 100644 --- a/Source/JavaScriptCore/runtime/JSModuleLoader.h +++ b/Source/JavaScriptCore/runtime/JSModuleLoader.h @@ -199,8 +199,10 @@ class JSModuleLoader final : public JSCell { // delete every (specifier, type) variant — text/json/HostDefined etc. // // A load of the removed entry may still be in flight. Its steps keep - // the detached entry, and isCacheableLoadedModule() keeps the record - // they produce out of m_loadedModules. Callers hold cellLock(). + // the detached entry (ModuleLoadingContext::setEntry for the top-level + // ones) and store its outcome there, and isCacheableLoadedModule() + // keeps the record they produce out of m_loadedModules. Callers hold + // cellLock(). auto* impl = key.impl(); m_loadedModules.removeIf([&](auto& entry) { return entry.key.first == impl; }); m_resolutionFailures.removeIf([&](auto& entry) { return entry.key.first == impl || entry.key.second == impl; }); diff --git a/Source/JavaScriptCore/runtime/ModuleLoadingContext.cpp b/Source/JavaScriptCore/runtime/ModuleLoadingContext.cpp index e619c0f64fd5..ca0b62252cbb 100644 --- a/Source/JavaScriptCore/runtime/ModuleLoadingContext.cpp +++ b/Source/JavaScriptCore/runtime/ModuleLoadingContext.cpp @@ -84,6 +84,13 @@ ModuleLoadingContext* ModuleLoadingContext::create(VM& vm, const AbstractModuleR return context; } +#if USE(BUN_JSC_ADDITIONS) +void ModuleLoadingContext::setEntry(VM& vm, ModuleRegistryEntry* entry) +{ + m_entry.set(vm, this, entry); +} +#endif + JSModuleLoader::ModuleReferrer ModuleLoadingContext::referrer() const { JSValue ref = m_referrer.get(); diff --git a/Source/JavaScriptCore/runtime/ModuleLoadingContext.h b/Source/JavaScriptCore/runtime/ModuleLoadingContext.h index 5de814a243a1..25d15b29ed55 100644 --- a/Source/JavaScriptCore/runtime/ModuleLoadingContext.h +++ b/Source/JavaScriptCore/runtime/ModuleLoadingContext.h @@ -70,6 +70,17 @@ class ModuleLoadingContext final : public JSCell { const AbstractModuleRecord::ModuleRequest& moduleRequest() const { return m_moduleRequest; } JSCell* payload() const { return m_payload.get(); } ModuleRegistryEntry* entry() const { return m_entry.get(); } +#if USE(BUN_JSC_ADDITIONS) + // A top-level load (second create() overload) has no entry until its fetch + // has settled and moduleLoadTopSettled has registered the key. The loader + // then records the entry the load goes on to load, and moduleLoadTopRejected + // and moduleLoadStoreError store the load's failure into that entry instead + // of into whatever entry the key holds by then. The two differ once the host + // has removed the entry while the load was in flight (removeEntry()): the key + // may by then hold the entry of a replacement load, which must not inherit + // the removed load's error. + void setEntry(VM&, ModuleRegistryEntry*); +#endif ScriptFetcher* scriptFetcher() const { return m_scriptFetcher.get(); } AbstractModuleRecord* module() const { return m_module.get(); } void module(VM& vm, AbstractModuleRecord* mod) { m_module.set(vm, this, mod); }