diff --git a/Source/JavaScriptCore/runtime/JSModuleLoader.cpp b/Source/JavaScriptCore/runtime/JSModuleLoader.cpp index 8f36cd3c96f5..94aa4729a482 100644 --- a/Source/JavaScriptCore/runtime/JSModuleLoader.cpp +++ b/Source/JavaScriptCore/runtime/JSModuleLoader.cpp @@ -638,7 +638,23 @@ JSPromise* JSModuleLoader::hostLoadImportedModule(JSGlobalObject* globalObject, // Consult it now so we can skip the host resolve() hook for repeat imports. { auto& loadedModules = record ? record->loadedModules() : m_loadedModules; - if (auto iter = loadedModules.find(moduleMapKey); iter != loadedModules.end()) { + auto iter = loadedModules.find(moduleMapKey); +#if USE(BUN_JSC_ADDITIONS) + // removeEntry() edits the registry underneath the realm-level cache, so the + // hit below cannot take what it asserts for granted there. removeEntry() + // takes a key and this cache is keyed by the request specifier, so when the + // host resolves the specifier to a different key the line outlives its entry: + // the registry may by now hold a new entry for the record's key (whose + // loadPromise may still be null) or none at all. Forget such a record and + // load what the registry holds. A module's own [[LoadedModules]] is not + // affected: innerModuleLoading consults it before calling here. + if (!record && iter != loadedModules.end() && !isCacheableLoadedModule(iter->value.m_module.get(), type)) { + Locker locker { cellLock() }; + loadedModules.remove(iter); + iter = loadedModules.end(); + } +#endif + if (iter != loadedModules.end()) { AbstractModuleRecord* loaded = iter->value.m_module.get(); ModuleRegistryEntry* loadedEntry = getRegisteredMayBeNull(loaded->moduleKey(), type); ASSERT(loadedEntry); @@ -958,8 +974,27 @@ void JSModuleLoader::finishLoadingImportedModule(JSGlobalObject* globalObject, c VM& vm = globalObject->vm(); auto scope = DECLARE_THROW_SCOPE(vm); +#if USE(BUN_JSC_ADDITIONS) + // A top-level load whose registry entry removeEntry() dropped while it was in + // flight still completes here (its ModuleLoadingContext holds the detached + // entry) and must still continue its payload, but its record has to stay out + // of the realm-level cache: the next import of the specifier has to load the + // key again, and a replacement load may already have cached its own record + // under the specifier, which step 1.a would assert against. A module's own + // [[LoadedModules]] is always filled in: it links against the record it got. + bool cacheRecord = true; + if (referrer.isRealm()) { + if (auto* resultRecord = std::get_if(&result)) + cacheRecord = isCacheableLoadedModule(*resultRecord, moduleRequest.type()); + } +#endif + // 1. If result is a normal completion, then +#if USE(BUN_JSC_ADDITIONS) + if (auto* resultRecord = cacheRecord ? std::get_if(&result) : nullptr) { +#else if (auto* resultRecord = std::get_if(&result)) { +#endif JSCell* owner = nullptr; auto& loadedModules = [&] -> ModuleMap & { @@ -1125,6 +1160,15 @@ ModuleRegistryEntry* JSModuleLoader::getRegisteredMayBeNull(const Identifier& ke } #if USE(BUN_JSC_ADDITIONS) +bool JSModuleLoader::isCacheableLoadedModule(AbstractModuleRecord* record, ScriptFetchParameters::Type type) +{ + // Mirrors what hostLoadImportedModule's cache hit asserts and returns: the + // entry's loadedPromise(), which exists for an entry that went through + // hostLoadImportedModule and for one the embedder markLoaded(). + ModuleRegistryEntry* entry = getRegisteredMayBeNull(record->moduleKey(), type); + return entry && entry->record() == record && (entry->loadPromise() || entry->isLoaded()); +} + int64_t JSModuleLoader::asyncEvaluationOrderForKey(const Identifier& key) { // For the deadlock-avoidance skip at innerModuleEvaluation step 12.b.v. diff --git a/Source/JavaScriptCore/runtime/JSModuleLoader.h b/Source/JavaScriptCore/runtime/JSModuleLoader.h index a422b279dd0f..bb3aa3778c91 100644 --- a/Source/JavaScriptCore/runtime/JSModuleLoader.h +++ b/Source/JavaScriptCore/runtime/JSModuleLoader.h @@ -197,6 +197,10 @@ class JSModuleLoader final : public JSCell { { // Bun's registry is conceptually flat (one entry per specifier), so // 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(). 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; }); @@ -221,6 +225,15 @@ class JSModuleLoader final : public JSCell { JSModuleLoader(VM&, Structure*); void finishCreation(JSGlobalObject*, VM&); +#if USE(BUN_JSC_ADDITIONS) + // hostLoadImportedModule() answers a hit in m_loadedModules with the registry + // entry of the cached record, so the cache may only hold records that the + // registry still holds, through an entry that has been loaded. removeEntry() + // can drop or replace an entry while a load of it is still in flight; both + // the hit and the insertion (finishLoadingImportedModule()) check this. + bool isCacheableLoadedModule(AbstractModuleRecord*, ScriptFetchParameters::Type); +#endif + void addResolutionFailure(VM&, const ResolutionMapKey&, JSValue error); // Corresponds to RealmRecord.[[LoadedModules]].