[module] Fix aborts in terminated async module evaluation

legendecas · V8 LUCI CQ · commit 22698d267667 · 2022-06-22T16:20:28.000Z
SourceTextModule::ExecuteAsyncModule asserts the execution of the module's async function to succeed without exception. However, the problem is that TerminateExecution initiated by embedders is breaking that assumption. The execution can be terminated with an exception and the exception is not catchable by JavaScript. The uncatchable exceptions during the async module evaluation need to be raised to the embedder and not crash the process if possible. Refs: nodejs/node#43182 Change-Id: Ifc152428b95945b6b49a2f70ba35018cfc0ce40b Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3696493 Reviewed-by: Camillo Bruni <cbruni@chromium.org> Commit-Queue: Chengzhong Wu <legendecas@gmail.com> Cr-Commit-Position: refs/heads/main@{#81307}
diff --git a/src/builtins/builtins-async-module.cc b/src/builtins/builtins-async-module.cc
@@ -15,7 +15,13 @@ BUILTIN(CallAsyncModuleFulfilled) {
       SourceTextModule::cast(isolate->context().get(
           SourceTextModule::ExecuteAsyncModuleContextSlots::kModule)),
       isolate);
-  SourceTextModule::AsyncModuleExecutionFulfilled(isolate, module);
+  if (SourceTextModule::AsyncModuleExecutionFulfilled(isolate, module)
+          .IsNothing()) {
+    // The evaluation of async module can not throwing a JavaScript observable
+    // exception.
+    DCHECK(isolate->is_execution_termination_pending());
+    return ReadOnlyRoots(isolate).exception();
+  }
   return ReadOnlyRoots(isolate).undefined_value();
 }
 
diff --git a/src/objects/source-text-module.cc b/src/objects/source-text-module.cc
@@ -749,14 +749,14 @@ MaybeHandle<Object> SourceTextModule::Evaluate(
   return capability;
 }
 
-void SourceTextModule::AsyncModuleExecutionFulfilled(
+Maybe<bool> SourceTextModule::AsyncModuleExecutionFulfilled(
     Isolate* isolate, Handle<SourceTextModule> module) {
   // 1. If module.[[Status]] is evaluated, then
   if (module->status() == kErrored) {
     // a. Assert: module.[[EvaluationError]] is not empty.
     DCHECK(!module->exception().IsTheHole(isolate));
     // b. Return.
-    return;
+    return Just(true);
   }
   // 3. Assert: module.[[AsyncEvaluating]] is true.
   DCHECK(module->IsAsyncEvaluating());
@@ -812,7 +812,9 @@ void SourceTextModule::AsyncModuleExecutionFulfilled(
     } else if (m->async()) {
       //  ii. Otherwise, if m.[[Async]] is *true*, then
       //   a. Perform ! ExecuteAsyncModule(m).
-      ExecuteAsyncModule(isolate, m);
+      // The execution may have been terminated and can not be resumed, so just
+      // raise the exception.
+      MAYBE_RETURN(ExecuteAsyncModule(isolate, m), Nothing<bool>());
     } else {
       //  iii. Otherwise,
       //   a. Let _result_ be m.ExecuteModule().
@@ -846,6 +848,7 @@ void SourceTextModule::AsyncModuleExecutionFulfilled(
   }
 
   // 10. Return undefined.
+  return Just(true);
 }
 
 void SourceTextModule::AsyncModuleExecutionRejected(
@@ -905,8 +908,9 @@ void SourceTextModule::AsyncModuleExecutionRejected(
   }
 }
 
-void SourceTextModule::ExecuteAsyncModule(Isolate* isolate,
-                                          Handle<SourceTextModule> module) {
+// static
+Maybe<bool> SourceTextModule::ExecuteAsyncModule(
+    Isolate* isolate, Handle<SourceTextModule> module) {
   // 1. Assert: module.[[Status]] is "evaluating" or "evaluated".
   CHECK(module->status() == kEvaluating || module->status() == kEvaluated);
 
@@ -961,9 +965,17 @@ void SourceTextModule::ExecuteAsyncModule(Isolate* isolate,
   // Note: In V8 we have broken module.ExecuteModule into
   // ExecuteModule for synchronous module execution and
   // InnerExecuteAsyncModule for asynchronous execution.
-  InnerExecuteAsyncModule(isolate, module, capability).ToHandleChecked();
+  MaybeHandle<Object> ret =
+      InnerExecuteAsyncModule(isolate, module, capability);
+  if (ret.is_null()) {
+    // The evaluation of async module can not throwing a JavaScript observable
+    // exception.
+    DCHECK(isolate->is_execution_termination_pending());
+    return Nothing<bool>();
+  }
 
   // 13. Return.
+  return Just<bool>(true);
 }
 
 MaybeHandle<Object> SourceTextModule::InnerExecuteAsyncModule(
@@ -1150,8 +1162,11 @@ MaybeHandle<Object> SourceTextModule::InnerModuleEvaluation(
 
     // c. If module.[[PendingAsyncDependencies]] is 0,
     //    perform ! ExecuteAsyncModule(_module_).
+    // The execution may have been terminated and can not be resumed, so just
+    // raise the exception.
     if (!module->HasPendingAsyncDependencies()) {
-      SourceTextModule::ExecuteAsyncModule(isolate, module);
+      MAYBE_RETURN(SourceTextModule::ExecuteAsyncModule(isolate, module),
+                   MaybeHandle<Object>());
     }
   } else {
     // 15. Otherwise, perform ? module.ExecuteModule().
diff --git a/src/objects/source-text-module.h b/src/objects/source-text-module.h
@@ -55,9 +55,10 @@ class SourceTextModule
   static int ExportIndex(int cell_index);
 
   // Used by builtins to fulfill or reject the promise associated
-  // with async SourceTextModules.
-  static void AsyncModuleExecutionFulfilled(Isolate* isolate,
-                                            Handle<SourceTextModule> module);
+  // with async SourceTextModules. Return Nothing if the execution is
+  // terminated.
+  static Maybe<bool> AsyncModuleExecutionFulfilled(
+      Isolate* isolate, Handle<SourceTextModule> module);
   static void AsyncModuleExecutionRejected(Isolate* isolate,
                                            Handle<SourceTextModule> module,
                                            Handle<Object> exception);
@@ -211,9 +212,10 @@ class SourceTextModule
   static V8_WARN_UNUSED_RESULT MaybeHandle<Object> ExecuteModule(
       Isolate* isolate, Handle<SourceTextModule> module);
 
-  // Implementation of spec ExecuteAsyncModule.
-  static void ExecuteAsyncModule(Isolate* isolate,
-                                 Handle<SourceTextModule> module);
+  // Implementation of spec ExecuteAsyncModule. Return Nothing if the execution
+  // is been terminated.
+  static V8_WARN_UNUSED_RESULT Maybe<bool> ExecuteAsyncModule(
+      Isolate* isolate, Handle<SourceTextModule> module);
 
   static void Reset(Isolate* isolate, Handle<SourceTextModule> module);
 
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
@@ -24368,6 +24368,122 @@ TEST(ImportFromSyntheticModuleThrow) {
   CHECK(try_catch.HasCaught());
 }
 
+namespace {
+
+v8::MaybeLocal<Module> ModuleEvaluateTerminateExecutionResolveCallback(
+    Local<Context> context, Local<String> specifier,
+    Local<FixedArray> import_assertions, Local<Module> referrer) {
+  v8::Isolate* isolate = context->GetIsolate();
+
+  Local<String> url = v8_str("www.test.com");
+  Local<String> source_text = v8_str("await Promise.resolve();");
+  v8::ScriptOrigin origin(isolate, url, 0, 0, false, -1, Local<v8::Value>(),
+                          false, false, true);
+  v8::ScriptCompiler::Source source(source_text, origin);
+  Local<Module> module =
+      v8::ScriptCompiler::CompileModule(isolate, &source).ToLocalChecked();
+  module
+      ->InstantiateModule(context,
+                          ModuleEvaluateTerminateExecutionResolveCallback)
+      .ToChecked();
+
+  CHECK_EQ(module->GetStatus(), Module::kInstantiated);
+  return module;
+}
+
+void ModuleEvaluateTerminateExecution(
+    const v8::FunctionCallbackInfo<v8::Value>& args) {
+  v8::Isolate::GetCurrent()->TerminateExecution();
+}
+}  // namespace
+
+TEST(ModuleEvaluateTerminateExecution) {
+  LocalContext env;
+  v8::Isolate* isolate = env->GetIsolate();
+  v8::Isolate::Scope iscope(isolate);
+  v8::HandleScope scope(isolate);
+  v8::Local<v8::Context> context = v8::Context::New(isolate);
+  v8::Context::Scope cscope(context);
+
+  v8::Local<v8::Function> terminate_execution =
+      v8::Function::New(context, ModuleEvaluateTerminateExecution,
+                        v8_str("terminate_execution"))
+          .ToLocalChecked();
+  context->Global()
+      ->Set(context, v8_str("terminate_execution"), terminate_execution)
+      .FromJust();
+
+  Local<String> url = v8_str("www.test.com");
+  Local<String> source_text = v8_str(
+      "terminate_execution();"
+      "await Promise.resolve();");
+  v8::ScriptOrigin origin(isolate, url, 0, 0, false, -1, Local<v8::Value>(),
+                          false, false, true);
+  v8::ScriptCompiler::Source source(source_text, origin);
+  Local<Module> module =
+      v8::ScriptCompiler::CompileModule(isolate, &source).ToLocalChecked();
+  module
+      ->InstantiateModule(context,
+                          ModuleEvaluateTerminateExecutionResolveCallback)
+      .ToChecked();
+
+  CHECK_EQ(module->GetStatus(), Module::kInstantiated);
+  TryCatch try_catch(isolate);
+  v8::MaybeLocal<Value> completion_value = module->Evaluate(context);
+  CHECK(completion_value.IsEmpty());
+
+  CHECK_EQ(module->GetStatus(), Module::kErrored);
+  CHECK(try_catch.HasCaught());
+  CHECK(try_catch.HasTerminated());
+}
+
+TEST(ModuleEvaluateImportTerminateExecution) {
+  LocalContext env;
+  v8::Isolate* isolate = env->GetIsolate();
+  v8::Isolate::Scope iscope(isolate);
+  v8::HandleScope scope(isolate);
+  v8::Local<v8::Context> context = v8::Context::New(isolate);
+  v8::Context::Scope cscope(context);
+
+  v8::Local<v8::Function> terminate_execution =
+      v8::Function::New(context, ModuleEvaluateTerminateExecution,
+                        v8_str("terminate_execution"))
+          .ToLocalChecked();
+  context->Global()
+      ->Set(context, v8_str("terminate_execution"), terminate_execution)
+      .FromJust();
+
+  Local<String> url = v8_str("www.test.com");
+  Local<String> source_text = v8_str(
+      "import './synthetic.module';"
+      "terminate_execution();"
+      "await Promise.resolve();");
+  v8::ScriptOrigin origin(isolate, url, 0, 0, false, -1, Local<v8::Value>(),
+                          false, false, true);
+  v8::ScriptCompiler::Source source(source_text, origin);
+  Local<Module> module =
+      v8::ScriptCompiler::CompileModule(isolate, &source).ToLocalChecked();
+  module
+      ->InstantiateModule(context,
+                          ModuleEvaluateTerminateExecutionResolveCallback)
+      .ToChecked();
+
+  CHECK_EQ(module->GetStatus(), Module::kInstantiated);
+  TryCatch try_catch(isolate);
+  v8::MaybeLocal<Value> completion_value = module->Evaluate(context);
+  Local<v8::Promise> promise(
+      Local<v8::Promise>::Cast(completion_value.ToLocalChecked()));
+  CHECK_EQ(promise->State(), v8::Promise::kPending);
+  isolate->PerformMicrotaskCheckpoint();
+
+  // The exception thrown by terminate execution is not catchable by JavaScript
+  // so the promise can not be settled.
+  CHECK_EQ(promise->State(), v8::Promise::kPending);
+  CHECK_EQ(module->GetStatus(), Module::kEvaluated);
+  CHECK(try_catch.HasCaught());
+  CHECK(try_catch.HasTerminated());
+}
+
 // Tests that the code cache does not confuse the same source code compiled as a
 // script and as a module.
 TEST(CodeCacheModuleScriptMismatch) {
