[modules] Fix bug in instantiation failure handling

GeorgNeis · Commit Bot · commit 6239ec101a4a · 2020-02-12T01:27:33.000Z
It's not sufficient to reset only the modules on the current DFS path. Bug: chromium:1050164 Change-Id: I00e5e12144ad70ac6371eea5e11590b72feaeecc Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2049853 Auto-Submit: Georg Neis <neis@chromium.org> Reviewed-by: Adam Klein <adamk@chromium.org> Commit-Queue: Adam Klein <adamk@chromium.org> Cr-Commit-Position: refs/heads/master@{#66229}
diff --git a/src/objects/module.cc b/src/objects/module.cc
@@ -76,9 +76,11 @@ void Module::RecordError(Isolate* isolate, Handle<Object> error) {
 }
 
 void Module::ResetGraph(Isolate* isolate, Handle<Module> module) {
-  DCHECK_NE(module->status(), kInstantiating);
   DCHECK_NE(module->status(), kEvaluating);
-  if (module->status() != kPreInstantiating) return;
+  if (module->status() != kPreInstantiating &&
+      module->status() != kInstantiating) {
+    return;
+  }
 
   Handle<FixedArray> requested_modules =
       module->IsSourceTextModule()
@@ -180,15 +182,14 @@ bool Module::Instantiate(Isolate* isolate, Handle<Module> module,
 
   if (!PrepareInstantiate(isolate, module, context, callback)) {
     ResetGraph(isolate, module);
+    DCHECK_EQ(module->status(), kUninstantiated);
     return false;
   }
   Zone zone(isolate->allocator(), ZONE_NAME);
   ZoneForwardList<Handle<SourceTextModule>> stack(&zone);
   unsigned dfs_index = 0;
   if (!FinishInstantiate(isolate, module, &stack, &dfs_index, &zone)) {
-    for (auto& descendant : stack) {
-      Reset(isolate, descendant);
-    }
+    ResetGraph(isolate, module);
     DCHECK_EQ(module->status(), kUninstantiated);
     return false;
   }
diff --git a/test/mjsunit/modules-reset.js b/test/mjsunit/modules-reset.js
@@ -0,0 +1,8 @@
+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax
+
+import("./modules-skip-reset1.js").then(() => %AbortJS("must throw"));
+import("./modules-skip-reset3.js").then(() => %AbortJS("must throw"));
diff --git a/test/mjsunit/modules-skip-reset1.js b/test/mjsunit/modules-skip-reset1.js
@@ -0,0 +1,7 @@
+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import {a} from "./modules-skip-reset2.js";
+import {b} from "./modules-skip-reset3.js";
+export let c = 42;
diff --git a/test/mjsunit/modules-skip-reset2.js b/test/mjsunit/modules-skip-reset2.js
@@ -0,0 +1,5 @@
+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+export {a} from "./modules-skip-reset2.js";
diff --git a/test/mjsunit/modules-skip-reset3.js b/test/mjsunit/modules-skip-reset3.js
@@ -0,0 +1,6 @@
+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import {c} from "./modules-skip-reset1.js";
+export let b = 23;
