From 8ee40cfc1fcc5e28eb981c61d8dcc523c6431307 Mon Sep 17 00:00:00 2001 From: Dan Clark Date: Thu, 13 Jan 2022 14:21:41 -0800 Subject: [PATCH] Don't double-fetch a module specified on the d8 command line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shell::FetchModuleTree assumes that the module at file_name wasn't already fetched. Shell::ExecuteModule is calling into FetchModuleTree without checking if the module is already in the module map, violating this assumption. This change fixes this by having Shell::ExecuteModule check for the existence of the module before calling into Shell::ExecuteModule, the same way that Shell::DoHostImportModuleDynamically does. Bug: v8:12530 Change-Id: Ia038cbd1715e85c9c92c4554fd486c657ef952e8 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3388130 Reviewed-by: Marja Hölttä Commit-Queue: Marja Hölttä Cr-Commit-Position: refs/heads/main@{#78636} --- src/d8/d8.cc | 12 ++++++++---- test/mjsunit/d8/d8-multiple-module-exec.js | 8 ++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) create mode 100644 test/mjsunit/d8/d8-multiple-module-exec.js diff --git a/src/d8/d8.cc b/src/d8/d8.cc index 16bc8946ea..e47f0d44a7 100644 --- a/src/d8/d8.cc +++ b/src/d8/d8.cc @@ -1365,11 +1365,15 @@ bool Shell::ExecuteModule(Isolate* isolate, const char* file_name) { // isolate->ReportPendingMessages(). TryCatch try_catch(isolate); + ModuleEmbedderData* d = GetModuleDataFromContext(realm); Local root_module; - - if (!FetchModuleTree(Local(), realm, absolute_path, - ModuleType::kJavaScript) - .ToLocal(&root_module)) { + auto module_it = d->module_map.find( + std::make_pair(absolute_path, ModuleType::kJavaScript)); + if (module_it != d->module_map.end()) { + root_module = module_it->second.Get(isolate); + } else if (!FetchModuleTree(Local(), realm, absolute_path, + ModuleType::kJavaScript) + .ToLocal(&root_module)) { CHECK(try_catch.HasCaught()); ReportException(isolate, &try_catch); return false; diff --git a/test/mjsunit/d8/d8-multiple-module-exec.js b/test/mjsunit/d8/d8-multiple-module-exec.js new file mode 100644 index 0000000000..6a6b6675e9 --- /dev/null +++ b/test/mjsunit/d8/d8-multiple-module-exec.js @@ -0,0 +1,8 @@ +// Copyright 2022 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: test/mjsunit/modules-skip-1.mjs test/mjsunit/modules-skip-1.mjs + +// Just test that d8 doesn't crash when running the same module on the +// command line twice.