Skip to content

Commit e3d8893

Browse files
legendecastargos
authored andcommitted
deps: V8: backport 22698d267667
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: #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: #43751 Backport-PR-URL: #44085 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Gus Caplan <[email protected]>
1 parent 0dc96e4 commit e3d8893

File tree

5 files changed

+158
-15
lines changed

5 files changed

+158
-15
lines changed

common.gypi

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636

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

4141
##### V8 defaults for Node.js #####
4242

deps/v8/src/builtins/builtins-async-module.cc

+9-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,15 @@ namespace internal {
1212
BUILTIN(CallAsyncModuleFulfilled) {
1313
HandleScope handle_scope(isolate);
1414
Handle<SourceTextModule> module(args.at<SourceTextModule>(0));
15-
SourceTextModule::AsyncModuleExecutionFulfilled(isolate, module);
15+
if (SourceTextModule::AsyncModuleExecutionFulfilled(isolate, module)
16+
.IsNothing()) {
17+
// The evaluation of async module can not throwing a JavaScript observable
18+
// exception.
19+
DCHECK(isolate->has_pending_exception());
20+
DCHECK_EQ(isolate->pending_exception(),
21+
ReadOnlyRoots(isolate).termination_exception());
22+
return ReadOnlyRoots(isolate).exception();
23+
}
1624
return ReadOnlyRoots(isolate).undefined_value();
1725
}
1826

deps/v8/src/objects/source-text-module.cc

+24-7
Original file line numberDiff line numberDiff line change
@@ -766,14 +766,14 @@ MaybeHandle<Object> SourceTextModule::Evaluate(
766766
return result;
767767
}
768768

769-
void SourceTextModule::AsyncModuleExecutionFulfilled(
769+
Maybe<bool> SourceTextModule::AsyncModuleExecutionFulfilled(
770770
Isolate* isolate, Handle<SourceTextModule> module) {
771771
// 1. If module.[[Status]] is evaluated, then
772772
if (module->status() == kErrored) {
773773
// a. Assert: module.[[EvaluationError]] is not empty.
774774
DCHECK(!module->exception().IsTheHole(isolate));
775775
// b. Return.
776-
return;
776+
return Just(true);
777777
}
778778
// 3. Assert: module.[[AsyncEvaluating]] is true.
779779
DCHECK(module->IsAsyncEvaluating());
@@ -829,7 +829,9 @@ void SourceTextModule::AsyncModuleExecutionFulfilled(
829829
} else if (m->async()) {
830830
// ii. Otherwise, if m.[[Async]] is *true*, then
831831
// a. Perform ! ExecuteAsyncModule(m).
832-
ExecuteAsyncModule(isolate, m);
832+
// The execution may have been terminated and can not be resumed, so just
833+
// raise the exception.
834+
MAYBE_RETURN(ExecuteAsyncModule(isolate, m), Nothing<bool>());
833835
} else {
834836
// iii. Otherwise,
835837
// a. Let _result_ be m.ExecuteModule().
@@ -863,6 +865,7 @@ void SourceTextModule::AsyncModuleExecutionFulfilled(
863865
}
864866

865867
// 10. Return undefined.
868+
return Just(true);
866869
}
867870

868871
void SourceTextModule::AsyncModuleExecutionRejected(
@@ -922,8 +925,9 @@ void SourceTextModule::AsyncModuleExecutionRejected(
922925
}
923926
}
924927

925-
void SourceTextModule::ExecuteAsyncModule(Isolate* isolate,
926-
Handle<SourceTextModule> module) {
928+
// static
929+
Maybe<bool> SourceTextModule::ExecuteAsyncModule(
930+
Isolate* isolate, Handle<SourceTextModule> module) {
927931
// 1. Assert: module.[[Status]] is "evaluating" or "evaluated".
928932
CHECK(module->status() == kEvaluating || module->status() == kEvaluated);
929933

@@ -973,9 +977,19 @@ void SourceTextModule::ExecuteAsyncModule(Isolate* isolate,
973977
// Note: In V8 we have broken module.ExecuteModule into
974978
// ExecuteModule for synchronous module execution and
975979
// InnerExecuteAsyncModule for asynchronous execution.
976-
InnerExecuteAsyncModule(isolate, module, capability).ToHandleChecked();
980+
MaybeHandle<Object> ret =
981+
InnerExecuteAsyncModule(isolate, module, capability);
982+
if (ret.is_null()) {
983+
// The evaluation of async module can not throwing a JavaScript observable
984+
// exception.
985+
DCHECK(isolate->has_pending_exception());
986+
DCHECK_EQ(isolate->pending_exception(),
987+
ReadOnlyRoots(isolate).termination_exception());
988+
return Nothing<bool>();
989+
}
977990

978991
// 13. Return.
992+
return Just<bool>(true);
979993
}
980994

981995
MaybeHandle<Object> SourceTextModule::InnerExecuteAsyncModule(
@@ -1171,8 +1185,11 @@ MaybeHandle<Object> SourceTextModule::InnerModuleEvaluation(
11711185

11721186
// c. If module.[[PendingAsyncDependencies]] is 0,
11731187
// perform ! ExecuteAsyncModule(_module_).
1188+
// The execution may have been terminated and can not be resumed, so just
1189+
// raise the exception.
11741190
if (!module->HasPendingAsyncDependencies()) {
1175-
SourceTextModule::ExecuteAsyncModule(isolate, module);
1191+
MAYBE_RETURN(SourceTextModule::ExecuteAsyncModule(isolate, module),
1192+
MaybeHandle<Object>());
11761193
}
11771194
} else {
11781195
// 15. Otherwise, perform ? module.ExecuteModule().

deps/v8/src/objects/source-text-module.h

+8-6
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,10 @@ class SourceTextModule
5353
static int ExportIndex(int cell_index);
5454

5555
// Used by builtins to fulfill or reject the promise associated
56-
// with async SourceTextModules.
57-
static void AsyncModuleExecutionFulfilled(Isolate* isolate,
58-
Handle<SourceTextModule> module);
56+
// with async SourceTextModules. Return Nothing if the execution is
57+
// terminated.
58+
static Maybe<bool> AsyncModuleExecutionFulfilled(
59+
Isolate* isolate, Handle<SourceTextModule> module);
5960
static void AsyncModuleExecutionRejected(Isolate* isolate,
6061
Handle<SourceTextModule> module,
6162
Handle<Object> exception);
@@ -204,9 +205,10 @@ class SourceTextModule
204205
static V8_WARN_UNUSED_RESULT MaybeHandle<Object> ExecuteModule(
205206
Isolate* isolate, Handle<SourceTextModule> module);
206207

207-
// Implementation of spec ExecuteAsyncModule.
208-
static void ExecuteAsyncModule(Isolate* isolate,
209-
Handle<SourceTextModule> module);
208+
// Implementation of spec ExecuteAsyncModule. Return Nothing if the execution
209+
// is been terminated.
210+
static V8_WARN_UNUSED_RESULT Maybe<bool> ExecuteAsyncModule(
211+
Isolate* isolate, Handle<SourceTextModule> module);
210212

211213
static void Reset(Isolate* isolate, Handle<SourceTextModule> module);
212214

deps/v8/test/cctest/test-api.cc

+116
Original file line numberDiff line numberDiff line change
@@ -24715,6 +24715,122 @@ TEST(ImportFromSyntheticModuleThrow) {
2471524715
CHECK(try_catch.HasCaught());
2471624716
}
2471724717

24718+
namespace {
24719+
24720+
v8::MaybeLocal<Module> ModuleEvaluateTerminateExecutionResolveCallback(
24721+
Local<Context> context, Local<String> specifier,
24722+
Local<FixedArray> import_assertions, Local<Module> referrer) {
24723+
v8::Isolate* isolate = context->GetIsolate();
24724+
24725+
Local<String> url = v8_str("www.test.com");
24726+
Local<String> source_text = v8_str("await Promise.resolve();");
24727+
v8::ScriptOrigin origin(isolate, url, 0, 0, false, -1, Local<v8::Value>(),
24728+
false, false, true);
24729+
v8::ScriptCompiler::Source source(source_text, origin);
24730+
Local<Module> module =
24731+
v8::ScriptCompiler::CompileModule(isolate, &source).ToLocalChecked();
24732+
module
24733+
->InstantiateModule(context,
24734+
ModuleEvaluateTerminateExecutionResolveCallback)
24735+
.ToChecked();
24736+
24737+
CHECK_EQ(module->GetStatus(), Module::kInstantiated);
24738+
return module;
24739+
}
24740+
24741+
void ModuleEvaluateTerminateExecution(
24742+
const v8::FunctionCallbackInfo<v8::Value>& args) {
24743+
v8::Isolate::GetCurrent()->TerminateExecution();
24744+
}
24745+
} // namespace
24746+
24747+
TEST(ModuleEvaluateTerminateExecution) {
24748+
LocalContext env;
24749+
v8::Isolate* isolate = env->GetIsolate();
24750+
v8::Isolate::Scope iscope(isolate);
24751+
v8::HandleScope scope(isolate);
24752+
v8::Local<v8::Context> context = v8::Context::New(isolate);
24753+
v8::Context::Scope cscope(context);
24754+
24755+
v8::Local<v8::Function> terminate_execution =
24756+
v8::Function::New(context, ModuleEvaluateTerminateExecution,
24757+
v8_str("terminate_execution"))
24758+
.ToLocalChecked();
24759+
context->Global()
24760+
->Set(context, v8_str("terminate_execution"), terminate_execution)
24761+
.FromJust();
24762+
24763+
Local<String> url = v8_str("www.test.com");
24764+
Local<String> source_text = v8_str(
24765+
"terminate_execution();"
24766+
"await Promise.resolve();");
24767+
v8::ScriptOrigin origin(isolate, url, 0, 0, false, -1, Local<v8::Value>(),
24768+
false, false, true);
24769+
v8::ScriptCompiler::Source source(source_text, origin);
24770+
Local<Module> module =
24771+
v8::ScriptCompiler::CompileModule(isolate, &source).ToLocalChecked();
24772+
module
24773+
->InstantiateModule(context,
24774+
ModuleEvaluateTerminateExecutionResolveCallback)
24775+
.ToChecked();
24776+
24777+
CHECK_EQ(module->GetStatus(), Module::kInstantiated);
24778+
TryCatch try_catch(isolate);
24779+
v8::MaybeLocal<Value> completion_value = module->Evaluate(context);
24780+
CHECK(completion_value.IsEmpty());
24781+
24782+
CHECK_EQ(module->GetStatus(), Module::kErrored);
24783+
CHECK(try_catch.HasCaught());
24784+
CHECK(try_catch.HasTerminated());
24785+
}
24786+
24787+
TEST(ModuleEvaluateImportTerminateExecution) {
24788+
LocalContext env;
24789+
v8::Isolate* isolate = env->GetIsolate();
24790+
v8::Isolate::Scope iscope(isolate);
24791+
v8::HandleScope scope(isolate);
24792+
v8::Local<v8::Context> context = v8::Context::New(isolate);
24793+
v8::Context::Scope cscope(context);
24794+
24795+
v8::Local<v8::Function> terminate_execution =
24796+
v8::Function::New(context, ModuleEvaluateTerminateExecution,
24797+
v8_str("terminate_execution"))
24798+
.ToLocalChecked();
24799+
context->Global()
24800+
->Set(context, v8_str("terminate_execution"), terminate_execution)
24801+
.FromJust();
24802+
24803+
Local<String> url = v8_str("www.test.com");
24804+
Local<String> source_text = v8_str(
24805+
"import './synthetic.module';"
24806+
"terminate_execution();"
24807+
"await Promise.resolve();");
24808+
v8::ScriptOrigin origin(isolate, url, 0, 0, false, -1, Local<v8::Value>(),
24809+
false, false, true);
24810+
v8::ScriptCompiler::Source source(source_text, origin);
24811+
Local<Module> module =
24812+
v8::ScriptCompiler::CompileModule(isolate, &source).ToLocalChecked();
24813+
module
24814+
->InstantiateModule(context,
24815+
ModuleEvaluateTerminateExecutionResolveCallback)
24816+
.ToChecked();
24817+
24818+
CHECK_EQ(module->GetStatus(), Module::kInstantiated);
24819+
TryCatch try_catch(isolate);
24820+
v8::MaybeLocal<Value> completion_value = module->Evaluate(context);
24821+
Local<v8::Promise> promise(
24822+
Local<v8::Promise>::Cast(completion_value.ToLocalChecked()));
24823+
CHECK_EQ(promise->State(), v8::Promise::kPending);
24824+
isolate->PerformMicrotaskCheckpoint();
24825+
24826+
// The exception thrown by terminate execution is not catchable by JavaScript
24827+
// so the promise can not be settled.
24828+
CHECK_EQ(promise->State(), v8::Promise::kPending);
24829+
CHECK_EQ(module->GetStatus(), Module::kEvaluated);
24830+
CHECK(try_catch.HasCaught());
24831+
CHECK(try_catch.HasTerminated());
24832+
}
24833+
2471824834
// Tests that the code cache does not confuse the same source code compiled as a
2471924835
// script and as a module.
2472024836
TEST(CodeCacheModuleScriptMismatch) {

0 commit comments

Comments
 (0)