diff --git a/Source/JavaScriptCore/runtime/JSModuleLoader.cpp b/Source/JavaScriptCore/runtime/JSModuleLoader.cpp index 22ba1ea14e9bd..e417c9a341a69 100644 --- a/Source/JavaScriptCore/runtime/JSModuleLoader.cpp +++ b/Source/JavaScriptCore/runtime/JSModuleLoader.cpp @@ -387,6 +387,16 @@ JSPromise* JSModuleLoader::loadModule(JSGlobalObject* globalObject, const Identi if (entry->status() != ModuleRegistryEntry::Status::New) promise = entry->ensureFetchPromise(globalObject); +#if USE(BUN_JSC_ADDITIONS) + // Beneath a synchronous load (loadModuleSync), the job that hands a delivered fetch to this promise + // can be parked in a queue that does not drain before this load has to complete (Bun: a macro's + // import() of a module that the require(esm) above it requested). Continue from the host's promise. + // hostLoadImportedModule() then settles the entry from it, as it does for a static request. + if (vm.m_synchronousModuleQueue) { + if (JSPromise* delivered = entry->deliveredFetch()) + promise = delivered; + } +#endif } } @@ -706,6 +716,24 @@ JSPromise* JSModuleLoader::hostLoadImportedModule(JSGlobalObject* globalObject, ModuleMapKey moduleMapKey { specifier.impl(), type }; +#if USE(BUN_JSC_ADDITIONS) + // An entry that has its record gets FinishLoadingImportedModule at once, and the caller gets the entry's + // load promise, which only the load that created the entry fulfills. Beneath a synchronous load + // (loadModuleSync), the reactions that do so can be parked in a queue that does not drain before this + // caller's load has to complete (Bun: a macro on the main thread waits for its import() inside + // require(esm)). A dynamic import does not need that promise: ContinueDynamicImport runs + // LoadRequestedModules on the record, then links and evaluates it. A top-level graph load (loadModule with + // a ModuleGraphLoadingState) links when that promise is fulfilled, so it keeps it. + auto loadPromiseAfterFinish = [&](JSPromise* entryLoadPromise, AbstractModuleRecord* loaded) -> JSPromise* { + if (!vm.m_synchronousModuleQueue || entryLoadPromise->status() != JSPromise::Status::Pending || !payload->inherits()) + return entryLoadPromise; + JSPromise* fulfilled = JSPromise::create(vm, globalObject->promiseStructure()); + fulfilled->markAsHandled(); + fulfilled->fulfill(vm, loaded); + return fulfilled; + }; +#endif + // HostLoadImportedModule is required to be idempotent for the same // (referrer, moduleRequest) pair. referrer.[[LoadedModules]] is that cache; // FinishLoadingImportedModule populates it, and innerModuleLoading consults it, @@ -726,7 +754,11 @@ JSPromise* JSModuleLoader::hostLoadImportedModule(JSGlobalObject* globalObject, ASSERT(loadedPromise); finishLoadingImportedModule(globalObject, referrer, moduleRequest, payload, loaded, scriptFetcher); RETURN_IF_EXCEPTION(scope, nullptr); +#if USE(BUN_JSC_ADDITIONS) + return loadPromiseAfterFinish(loadedPromise, loaded); +#else return loadedPromise; +#endif } } @@ -810,19 +842,24 @@ JSPromise* JSModuleLoader::hostLoadImportedModule(JSGlobalObject* globalObject, JSPromise* modulePromise = mapEntry->ensureModulePromise(globalObject); if (modulePromise->status() == JSPromise::Status::Pending) { if (fetchPromise->status() == JSPromise::Status::Pending) { - // Transpilation still in flight — re-issue through the - // embedder's synchronous fetch. fetchPromise was already - // pipeFrom()'d by the async path which set - // isFirstResolvingFunctionCalledFlag, so use the unguarded - // fulfill/reject. The ModuleRegistryFetchSettled reaction on - // fetchPromise lands on the sync queue and drives the rest of - // the chain (including loadPromise). - JSPromise* promise = fetch(globalObject, identifierToJSValue(vm, resolved), moduleReferrer(referrerKey), nullptr, scriptFetcher.copyRef()); - RETURN_IF_EXCEPTION(scope, nullptr); - if (promise->status() == JSPromise::Status::Fulfilled) - fetchPromise->fulfillPromise(vm, promise->result()); - else if (promise->status() == JSPromise::Status::Rejected) - fetchPromise->rejectPromise(vm, promise->result()); + // The host has often delivered this fetch already: beneath a synchronous load it + // fetches synchronously, and only the pipeFrom() job that hands the result to + // fetchPromise is parked in a queue. Fetch a second time only if it has not. + if (!mapEntry->takeSettledFetchSource(vm)) { + // Transpilation still in flight — re-issue through the + // embedder's synchronous fetch. fetchPromise was already + // pipeFrom()'d by the async path which set + // isFirstResolvingFunctionCalledFlag, so use the unguarded + // fulfill/reject. The ModuleRegistryFetchSettled reaction on + // fetchPromise lands on the sync queue and drives the rest of + // the chain (including loadPromise). + JSPromise* promise = fetch(globalObject, identifierToJSValue(vm, resolved), moduleReferrer(referrerKey), nullptr, scriptFetcher.copyRef()); + RETURN_IF_EXCEPTION(scope, nullptr); + if (promise->status() == JSPromise::Status::Fulfilled) + fetchPromise->fulfillPromise(vm, promise->result()); + else if (promise->status() == JSPromise::Status::Rejected) + fetchPromise->rejectPromise(vm, promise->result()); + } } else if (fetchPromise->status() == JSPromise::Status::Fulfilled) { // fetchPromise already settled but its // ModuleRegistryFetchSettled reaction is sitting on the @@ -854,9 +891,12 @@ JSPromise* JSModuleLoader::hostLoadImportedModule(JSGlobalObject* globalObject, JSPromise* promise = mapEntry->loadPromise(); #endif if (promise) { - if (mapEntry->record()) { - finishLoadingImportedModule(globalObject, referrer, moduleRequest, payload, mapEntry->record(), scriptFetcher); + if (AbstractModuleRecord* loaded = mapEntry->record()) { + finishLoadingImportedModule(globalObject, referrer, moduleRequest, payload, loaded, scriptFetcher); RETURN_IF_EXCEPTION(scope, nullptr); +#if USE(BUN_JSC_ADDITIONS) + promise = loadPromiseAfterFinish(promise, loaded); +#endif } else { auto* context = ModuleLoadingContext::create(vm, this, ModuleLoadingContext::Step::Cached, referrer, moduleRequest, payload, mapEntry, scriptFetcher); JSPromise* resultPromise = JSPromise::create(vm, globalObject->promiseStructure()); @@ -881,6 +921,9 @@ JSPromise* JSModuleLoader::hostLoadImportedModule(JSGlobalObject* globalObject, mapEntry->setStatus(ModuleRegistryEntry::Status::Fetching); mapEntry->ensureFetchPromise(globalObject)->pipeFrom(vm, promise); +#if USE(BUN_JSC_ADDITIONS) + mapEntry->setFetchSource(vm, promise); +#endif } JSPromise* modulePromise = mapEntry->ensureModulePromise(globalObject); RETURN_IF_EXCEPTION(scope, nullptr); @@ -981,7 +1024,6 @@ void JSModuleLoader::innerModuleLoading(JSGlobalObject* globalObject, ModuleGrap } // 2.d.iii. Else, // 2.d.iii.1. Perform HostLoadImportedModule(module, request, state.[[HostDefined]], state). - unsigned loadedModulesCountBefore = module->loadedModules().size(); JSPromise* promise = hostLoadImportedModule(globalObject, cyclic, request, state, state->scriptFetcher(), true); if (scope.exception()) [[unlikely]] { state->setDrainingInnerLoad(false); @@ -989,12 +1031,18 @@ void JSModuleLoader::innerModuleLoading(JSGlobalObject* globalObject, ModuleGrap } // 2.d.iii.2. NOTE: HostLoadImportedModule will call FinishLoadingImportedModule, which re-enters the graph loading process through ContinueModuleLoading. // - // If module.[[LoadedModules]] grew across the HostLoadImportedModule call, the requested - // module was loaded synchronously, which means it was already loaded before. In that case - // there is no need to attach a ModuleGraphLoadingError reaction, so we skip it. - bool needsErrorReaction = module->loadedModules().size() == loadedModulesCountBefore; - ASSERT(module->loadedModules().size() <= loadedModulesCountBefore + 1); - ASSERT(needsErrorReaction != module->loadedModules().contains(ModuleMapKey { request.m_specifier.impl(), request.type() })); + // If the requested module is in module.[[LoadedModules]] after the + // HostLoadImportedModule call, it was loaded synchronously and + // FinishLoadingImportedModule already continued this state, so no + // ModuleGraphLoadingError reaction is needed. Membership — not a + // loadedModules size delta — is the test: a require(esm) nested + // inside the host hook (a CommonJS module evaluated during + // makeModule requiring a sibling of this graph) drains the + // synchronous module queue, which can run this module's pending + // ModuleLoadStep reactions for OTHER requests during the call, so + // the size can grow without THIS request having completed — and + // grow by more than one. + bool needsErrorReaction = !module->loadedModules().contains(ModuleMapKey { request.m_specifier.impl(), request.type() }); if (needsErrorReaction) promise->performPromiseThenWithInternalMicrotask(vm, InternalMicrotask::ModuleGraphLoadingError, nullptr, state); // 2.d.iv. If state.[[IsLoading]] is false, return UNUSED. diff --git a/Source/JavaScriptCore/runtime/ModuleRegistryEntry.cpp b/Source/JavaScriptCore/runtime/ModuleRegistryEntry.cpp index eaafa5340e054..fa9ca50f30f33 100644 --- a/Source/JavaScriptCore/runtime/ModuleRegistryEntry.cpp +++ b/Source/JavaScriptCore/runtime/ModuleRegistryEntry.cpp @@ -69,6 +69,9 @@ void ModuleRegistryEntry::visitChildrenImpl(JSCell* cell, Visitor& visitor) visitor.append(thisObject->m_modulePromise); visitor.append(thisObject->m_loadPromise); visitor.append(thisObject->m_error); +#if USE(BUN_JSC_ADDITIONS) + visitor.append(thisObject->m_fetchSource); +#endif } DEFINE_VISIT_CHILDREN(ModuleRegistryEntry); @@ -224,6 +227,10 @@ void ModuleRegistryEntry::setEvaluationError(JSGlobalObject* globalObject, JSVal void ModuleRegistryEntry::setStatus(Status status) { m_status = status; +#if USE(BUN_JSC_ADDITIONS) + if (status != Status::New && status != Status::Fetching) + m_fetchSource.clear(); +#endif } void ModuleRegistryEntry::provideFetch(JSGlobalObject* globalObject, SourceCode&& sourceCode) @@ -253,6 +260,7 @@ void ModuleRegistryEntry::provideModule(VM& vm, AbstractModuleRecord* record) ASSERT(m_status == Status::New || m_status == Status::Fetching); m_record.set(vm, this, record); m_status = Status::Fetched; + m_fetchSource.clear(); // Promises some in-flight load already created for this entry are settled now (with the record standing in for // the source: moduleLoadTopSettled has nothing to provide for it); their pending reactions see the settled module // promise and bail. @@ -267,6 +275,34 @@ bool ModuleRegistryEntry::isLoaded() const return m_isLoaded || (m_loadPromise && m_loadPromise->status() == JSPromise::Status::Fulfilled); } +void ModuleRegistryEntry::setFetchSource(VM& vm, JSPromise* source) +{ + m_fetchSource.set(vm, this, source); +} + +JSPromise* ModuleRegistryEntry::deliveredFetch() const +{ + JSPromise* source = m_fetchSource.get(); + if (!source || source->status() == JSPromise::Status::Pending || !m_fetchPromise || m_fetchPromise->status() != JSPromise::Status::Pending) + return nullptr; + return source; +} + +bool ModuleRegistryEntry::takeSettledFetchSource(VM& vm) +{ + JSPromise* source = deliveredFetch(); + if (!source) + return false; + m_fetchSource.clear(); + // m_fetchPromise was pipeFrom()'d, so only the unguarded forms settle it. The pipe's job finds it settled later + // and does nothing. + if (source->status() == JSPromise::Status::Fulfilled) + m_fetchPromise->fulfillPromise(vm, source->result()); + else + m_fetchPromise->rejectPromise(vm, source->result()); + return true; +} + JSPromise* ModuleRegistryEntry::loadedPromise(JSGlobalObject* globalObject) { if (!m_loadPromise && m_isLoaded) { @@ -290,6 +326,9 @@ void ModuleRegistryEntry::fetchComplete(JSGlobalObject* globalObject, AbstractMo ASSERT(m_status == Status::Fetching); m_record.set(vm, this, record); m_status = Status::Fetched; +#if USE(BUN_JSC_ADDITIONS) + m_fetchSource.clear(); +#endif } } // namespace JSC diff --git a/Source/JavaScriptCore/runtime/ModuleRegistryEntry.h b/Source/JavaScriptCore/runtime/ModuleRegistryEntry.h index 3c535c70d6798..0c6ff866b120b 100644 --- a/Source/JavaScriptCore/runtime/ModuleRegistryEntry.h +++ b/Source/JavaScriptCore/runtime/ModuleRegistryEntry.h @@ -102,6 +102,14 @@ class ModuleRegistryEntry final : public JSCell { bool isLoaded() const; // loadPromise(), materializing it first for an entry that was markLoaded(). JSPromise* loadedPromise(JSGlobalObject*); + // The host's fetch promise that pipeFrom() connects to the fetch promise. The connection is a microtask, so the + // host can have delivered while the fetch promise is still pending. A synchronous load (loadModuleSync) that + // cannot wait for that microtask uses the delivered result, and does not fetch again. + void setFetchSource(VM&, JSPromise*); + // The fetch source, when it is settled and the fetch promise is still pending. Null otherwise. + JSPromise* deliveredFetch() const; + // Settles the fetch promise from deliveredFetch(). Returns false when there is none. + bool takeSettledFetchSource(VM&); #endif private: @@ -123,6 +131,7 @@ class ModuleRegistryEntry final : public JSCell { Status m_status { Status::New }; #if USE(BUN_JSC_ADDITIONS) bool m_isLoaded { false }; + WriteBarrier m_fetchSource; #endif };