Skip to content

Commit

Permalink
deps: V8: backport 22698d267667
Browse files Browse the repository at this point in the history
Original commit message:

    [module] Fix aborts in terminated async module evaluation

    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 <[email protected]>
    Commit-Queue: Chengzhong Wu <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#81307}

Refs: v8/v8@22698d2
PR-URL: nodejs/node#43751
Backport-PR-URL: nodejs/node#44085
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
  • Loading branch information
legendecas authored and guangwong committed Oct 10, 2022
1 parent 64da369 commit 0e86d89
Show file tree
Hide file tree
Showing 5 changed files with 158 additions and 15 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.21',
'v8_embedder_string': '-node.22',

##### V8 defaults for Node.js #####

Expand Down
10 changes: 9 additions & 1 deletion deps/v8/src/builtins/builtins-async-module.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,15 @@ namespace internal {
BUILTIN(CallAsyncModuleFulfilled) {
HandleScope handle_scope(isolate);
Handle<SourceTextModule> module(args.at<SourceTextModule>(0));
SourceTextModule::AsyncModuleExecutionFulfilled(isolate, module);
if (SourceTextModule::AsyncModuleExecutionFulfilled(isolate, module)
.IsNothing()) {
// The evaluation of async module can not throwing a JavaScript observable
// exception.
DCHECK(isolate->has_pending_exception());
DCHECK_EQ(isolate->pending_exception(),
ReadOnlyRoots(isolate).termination_exception());
return ReadOnlyRoots(isolate).exception();
}
return ReadOnlyRoots(isolate).undefined_value();
}

Expand Down
31 changes: 24 additions & 7 deletions deps/v8/src/objects/source-text-module.cc
Original file line number Diff line number Diff line change
Expand Up @@ -766,14 +766,14 @@ MaybeHandle<Object> SourceTextModule::Evaluate(
return result;
}

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());
Expand Down Expand Up @@ -829,7 +829,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().
Expand Down Expand Up @@ -863,6 +865,7 @@ void SourceTextModule::AsyncModuleExecutionFulfilled(
}

// 10. Return undefined.
return Just(true);
}

void SourceTextModule::AsyncModuleExecutionRejected(
Expand Down Expand Up @@ -922,8 +925,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);

Expand Down Expand Up @@ -973,9 +977,19 @@ 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->has_pending_exception());
DCHECK_EQ(isolate->pending_exception(),
ReadOnlyRoots(isolate).termination_exception());
return Nothing<bool>();
}

// 13. Return.
return Just<bool>(true);
}

MaybeHandle<Object> SourceTextModule::InnerExecuteAsyncModule(
Expand Down Expand Up @@ -1171,8 +1185,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().
Expand Down
14 changes: 8 additions & 6 deletions deps/v8/src/objects/source-text-module.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,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);
Expand Down Expand Up @@ -204,9 +205,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);

Expand Down
116 changes: 116 additions & 0 deletions deps/v8/test/cctest/test-api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24715,6 +24715,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) {
Expand Down

0 comments on commit 0e86d89

Please sign in to comment.