diff --git a/lib/Runtime/Language/SourceTextModuleRecord.cpp b/lib/Runtime/Language/SourceTextModuleRecord.cpp index b65091a3792..495c19891c0 100644 --- a/lib/Runtime/Language/SourceTextModuleRecord.cpp +++ b/lib/Runtime/Language/SourceTextModuleRecord.cpp @@ -42,7 +42,6 @@ namespace Js isRootModule(false), hadNotifyHostReady(false), localExportSlots(nullptr), - numPendingChildrenModule(0), moduleId(InvalidModuleIndex), localSlotCount(InvalidSlotCount), promise(nullptr), @@ -225,6 +224,11 @@ namespace Js void SourceTextModuleRecord::NotifyParentsAsNeeded() { + if (notifying) + { + return; + } + notifying = true; // Notify the parent modules that this child module is either in fault state or finished. if (this->parentModuleList != nullptr) { @@ -233,6 +237,7 @@ namespace Js parentModule->OnChildModuleReady(this, this->errorObject); }); } + notifying = false; SetParentsNotified(); } @@ -326,7 +331,7 @@ namespace Js OUTPUT_TRACE_DEBUGONLY(Js::ModulePhase, _u("PrepareForModuleDeclarationInitialization(%s)\n"), this->GetSpecifierSz()); HRESULT hr = NO_ERROR; - if (numPendingChildrenModule == 0) + if (ConfirmChildrenParsed()) { OUTPUT_TRACE_DEBUGONLY(Js::ModulePhase, _u("\t>NotifyParentsAsNeeded\n")); NotifyParentsAsNeeded(); @@ -355,12 +360,6 @@ namespace Js } } } -#if DBG - else - { - OUTPUT_TRACE_DEBUGONLY(Js::ModulePhase, _u("\tnumPendingChildrenModule=(%d)\n"), numPendingChildrenModule); - } -#endif return hr; } @@ -399,12 +398,6 @@ namespace Js } else { - if (numPendingChildrenModule == 0) - { - return NOERROR; // this is only in case of recursive module reference. Let the higher stack frame handle this module. - } - numPendingChildrenModule--; - hr = PrepareForModuleDeclarationInitialization(); } return hr; @@ -715,21 +708,52 @@ namespace Js { parentRecord->childrenModuleSet->AddNew(moduleName, this); - if (!this->WasParsed()) + if (this->parentModuleList == nullptr) { - if (this->parentModuleList == nullptr) + Recycler* recycler = GetScriptContext()->GetRecycler(); + this->parentModuleList = RecyclerNew(recycler, ModuleRecordList, recycler); + } + + if (!this->parentModuleList->Contains(parentRecord)) + { + this->parentModuleList->Add(parentRecord); + } + } + } + + bool SourceTextModuleRecord::ConfirmChildrenParsed() + { + if (!this->WasParsed()) + { + return false; + } + if (confirmedReady || this->ParentsNotified()) + { + return true; + } + bool result = true; + confirmedReady = true; + EnsureChildModuleSet(GetScriptContext()); + childrenModuleSet->EachValue([&](SourceTextModuleRecord* childModuleRecord) { + if (childModuleRecord->ParentsNotified()) + { + return false; + } + else + { + if (childModuleRecord->ConfirmChildrenParsed()) { - Recycler* recycler = GetScriptContext()->GetRecycler(); - this->parentModuleList = RecyclerNew(recycler, ModuleRecordList, recycler); + return false; } - - if (!this->parentModuleList->Contains(parentRecord)) + else { - this->parentModuleList->Add(parentRecord); - parentRecord->numPendingChildrenModule++; + result = false; + return true; } } - } + }); + confirmedReady = false; + return result; } void SourceTextModuleRecord::EnsureChildModuleSet(ScriptContext *scriptContext) @@ -751,16 +775,9 @@ namespace Js if (requestedModuleList != nullptr) { EnsureChildModuleSet(scriptContext); - ArenaAllocator* allocator = scriptContext->GeneralAllocator(); - SList * moduleRecords = Anew(allocator, SList, allocator); - // Reverse the order for the host. So, host can read the files top-down requestedModuleList->MapUntil([&](IdentPtr specifier) { LPCOLESTR moduleName = specifier->Psz(); - return !moduleRecords->Prepend(moduleName); - }); - - moduleRecords->MapUntil([&](LPCOLESTR moduleName) { ModuleRecordBase* moduleRecordBase = nullptr; SourceTextModuleRecord* moduleRecord = nullptr; bool itemFound = childrenModuleSet->TryGetValue(moduleName, &moduleRecord); @@ -787,7 +804,6 @@ namespace Js } return false; }); - moduleRecords->Clear(); if (FAILED(hr)) { diff --git a/lib/Runtime/Language/SourceTextModuleRecord.h b/lib/Runtime/Language/SourceTextModuleRecord.h index 7e96fcd913f..2261ff23df2 100644 --- a/lib/Runtime/Language/SourceTextModuleRecord.h +++ b/lib/Runtime/Language/SourceTextModuleRecord.h @@ -43,6 +43,7 @@ namespace Js HRESULT ResolveExternalModuleDependencies(); void EnsureChildModuleSet(ScriptContext *scriptContext); + bool ConfirmChildrenParsed(); void* GetHostDefined() const { return hostDefined; } void SetHostDefined(void* hostObj) { hostDefined = hostObj; } @@ -116,6 +117,8 @@ namespace Js const static uint InvalidSlotIndex = 0xffffffff; // TODO: move non-GC fields out to avoid false reference? // This is the parsed tree resulted from compilation. + Field(bool) confirmedReady = false; + Field(bool) notifying = false; Field(bool) wasParsed; Field(bool) wasDeclarationInitialized; Field(bool) parentsNotified; @@ -136,7 +139,6 @@ namespace Js Field(LocalExportMap*) localExportMapByExportName; // from propertyId to index map: for bytecode gen. Field(LocalExportMap*) localExportMapByLocalName; // from propertyId to index map: for bytecode gen. Field(LocalExportIndexList*) localExportIndexList; // from index to propertyId: for typehandler. - Field(uint) numPendingChildrenModule; Field(ExportedNames*) exportedNames; Field(ResolvedExportMap*) resolvedExportMap; diff --git a/test/es6module/module-bugfixes.js b/test/es6module/module-bugfixes.js index 9974193f118..13d0ab3330c 100644 --- a/test/es6module/module-bugfixes.js +++ b/test/es6module/module-bugfixes.js @@ -67,6 +67,32 @@ var tests = [ let functionBody = "import 'module_4570_dep1.js';" testRunner.LoadModule(functionBody); } + }, + { + name: "Issue 5171: Incorrect module load order", + body: function() { + WScript.RegisterModuleSource("obj.js", `export const obj = {a:false, b: false, c: false};`); + WScript.RegisterModuleSource("a.js",` + import {obj} from "obj.js"; + assert.isTrue(obj.b); + assert.isFalse(obj.c); + assert.isFalse(obj.a); + obj.a = true;`); + WScript.RegisterModuleSource("b.js",` + import {obj} from "obj.js"; + assert.isFalse(obj.b); + assert.isFalse(obj.c); + assert.isFalse(obj.a); + obj.b = true;`); + WScript.RegisterModuleSource("c.js",` + import {obj} from "obj.js"; + assert.isTrue(obj.b); + assert.isFalse(obj.c); + assert.isTrue(obj.a); + obj.c = true;`); + const start = 'import "b.js"; import "a.js"; import "c.js";'; + testRunner.LoadModule(start); + } } ]; diff --git a/test/es6module/multiple-roots-circular.js b/test/es6module/multiple-roots-circular.js new file mode 100644 index 00000000000..7094a609fc5 --- /dev/null +++ b/test/es6module/multiple-roots-circular.js @@ -0,0 +1,15 @@ +//------------------------------------------------------------------------------------------------------- +// Copyright (C) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. +//------------------------------------------------------------------------------------------------------- + +// Tests that circular overlapping module dependencies are all loaded before execution + +WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js"); + +WScript.RegisterModuleSource("mod0.js", "print('pass')"); +WScript.RegisterModuleSource("mod1.js", "import 'mod2.js';"); +WScript.RegisterModuleSource("mod2.js", "import 'mod0.js';"); + +WScript.LoadScriptFile("mod2.js", "module"); +WScript.LoadScriptFile("mod1.js", "module"); diff --git a/test/es6module/rlexe.xml b/test/es6module/rlexe.xml index 214209bbea6..e1e39b1ff09 100644 --- a/test/es6module/rlexe.xml +++ b/test/es6module/rlexe.xml @@ -139,6 +139,12 @@ bug_issue_3257.baseline exclude_sanitize_address + + + + multiple-roots-circular.js + exclude_sanitize_address +