Skip to content

Commit 2e1732e

Browse files
legendecasdanielleadams
legendecas
authored and
danielleadams
committed
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 <cbruni@chromium.org> Commit-Queue: Chengzhong Wu <legendecas@gmail.com> Cr-Commit-Position: refs/heads/main@{#81307} Refs: v8/v8@22698d2 PR-URL: #43751 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Gus Caplan <me@gus.host>
1 parent ceb7c15 commit 2e1732e

File tree

5 files changed

+158
-15
lines changed

5 files changed

+158
-15
lines changed

‎common.gypi

Lines changed: 1 addition & 1 deletion
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.8',
39+
'v8_embedder_string': '-node.9',
4040

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

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

Lines changed: 9 additions & 1 deletion
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

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -749,14 +749,14 @@ MaybeHandle<Object> SourceTextModule::Evaluate(
749749
return capability;
750750
}
751751

752-
void SourceTextModule::AsyncModuleExecutionFulfilled(
752+
Maybe<bool> SourceTextModule::AsyncModuleExecutionFulfilled(
753753
Isolate* isolate, Handle<SourceTextModule> module) {
754754
// 1. If module.[[Status]] is evaluated, then
755755
if (module->status() == kErrored) {
756756
// a. Assert: module.[[EvaluationError]] is not empty.
757757
DCHECK(!module->exception().IsTheHole(isolate));
758758
// b. Return.
759-
return;
759+
return Just(true);
760760
}
761761
// 3. Assert: module.[[AsyncEvaluating]] is true.
762762
DCHECK(module->IsAsyncEvaluating());
@@ -812,7 +812,9 @@ void SourceTextModule::AsyncModuleExecutionFulfilled(
812812
} else if (m->async()) {
813813
// ii. Otherwise, if m.[[Async]] is *true*, then
814814
// a. Perform ! ExecuteAsyncModule(m).
815-
ExecuteAsyncModule(isolate, m);
815+
// The execution may have been terminated and can not be resumed, so just
816+
// raise the exception.
817+
MAYBE_RETURN(ExecuteAsyncModule(isolate, m), Nothing<bool>());
816818
} else {
817819
// iii. Otherwise,
818820
// a. Let _result_ be m.ExecuteModule().
@@ -846,6 +848,7 @@ void SourceTextModule::AsyncModuleExecutionFulfilled(
846848
}
847849

848850
// 10. Return undefined.
851+
return Just(true);
849852
}
850853

851854
void SourceTextModule::AsyncModuleExecutionRejected(
@@ -905,8 +908,9 @@ void SourceTextModule::AsyncModuleExecutionRejected(
905908
}
906909
}
907910

908-
void SourceTextModule::ExecuteAsyncModule(Isolate* isolate,
909-
Handle<SourceTextModule> module) {
911+
// static
912+
Maybe<bool> SourceTextModule::ExecuteAsyncModule(
913+
Isolate* isolate, Handle<SourceTextModule> module) {
910914
// 1. Assert: module.[[Status]] is "evaluating" or "evaluated".
911915
CHECK(module->status() == kEvaluating || module->status() == kEvaluated);
912916

@@ -956,9 +960,19 @@ void SourceTextModule::ExecuteAsyncModule(Isolate* isolate,
956960
// Note: In V8 we have broken module.ExecuteModule into
957961
// ExecuteModule for synchronous module execution and
958962
// InnerExecuteAsyncModule for asynchronous execution.
959-
InnerExecuteAsyncModule(isolate, module, capability).ToHandleChecked();
963+
MaybeHandle<Object> ret =
964+
InnerExecuteAsyncModule(isolate, module, capability);
965+
if (ret.is_null()) {
966+
// The evaluation of async module can not throwing a JavaScript observable
967+
// exception.
968+
DCHECK(isolate->has_pending_exception());
969+
DCHECK_EQ(isolate->pending_exception(),
970+
ReadOnlyRoots(isolate).termination_exception());
971+
return Nothing<bool>();
972+
}
960973

961974
// 13. Return.
975+
return Just<bool>(true);
962976
}
963977

964978
MaybeHandle<Object> SourceTextModule::InnerExecuteAsyncModule(
@@ -1145,8 +1159,11 @@ MaybeHandle<Object> SourceTextModule::InnerModuleEvaluation(
11451159

11461160
// c. If module.[[PendingAsyncDependencies]] is 0,
11471161
// perform ! ExecuteAsyncModule(_module_).
1162+
// The execution may have been terminated and can not be resumed, so just
1163+
// raise the exception.
11481164
if (!module->HasPendingAsyncDependencies()) {
1149-
SourceTextModule::ExecuteAsyncModule(isolate, module);
1165+
MAYBE_RETURN(SourceTextModule::ExecuteAsyncModule(isolate, module),
1166+
MaybeHandle<Object>());
11501167
}
11511168
} else {
11521169
// 15. Otherwise, perform ? module.ExecuteModule().

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,10 @@ class SourceTextModule
5454
static int ExportIndex(int cell_index);
5555

5656
// Used by builtins to fulfill or reject the promise associated
57-
// with async SourceTextModules.
58-
static void AsyncModuleExecutionFulfilled(Isolate* isolate,
59-
Handle<SourceTextModule> module);
57+
// with async SourceTextModules. Return Nothing if the execution is
58+
// terminated.
59+
static Maybe<bool> AsyncModuleExecutionFulfilled(
60+
Isolate* isolate, Handle<SourceTextModule> module);
6061
static void AsyncModuleExecutionRejected(Isolate* isolate,
6162
Handle<SourceTextModule> module,
6263
Handle<Object> exception);
@@ -201,9 +202,10 @@ class SourceTextModule
201202
static V8_WARN_UNUSED_RESULT MaybeHandle<Object> ExecuteModule(
202203
Isolate* isolate, Handle<SourceTextModule> module);
203204

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

208210
static void Reset(Isolate* isolate, Handle<SourceTextModule> module);
209211

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

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24649,6 +24649,122 @@ TEST(ImportFromSyntheticModuleThrow) {
2464924649
CHECK(try_catch.HasCaught());
2465024650
}
2465124651

24652+
namespace {
24653+
24654+
v8::MaybeLocal<Module> ModuleEvaluateTerminateExecutionResolveCallback(
24655+
Local<Context> context, Local<String> specifier,
24656+
Local<FixedArray> import_assertions, Local<Module> referrer) {
24657+
v8::Isolate* isolate = context->GetIsolate();
24658+
24659+
Local<String> url = v8_str("www.test.com");
24660+
Local<String> source_text = v8_str("await Promise.resolve();");
24661+
v8::ScriptOrigin origin(isolate, url, 0, 0, false, -1, Local<v8::Value>(),
24662+
false, false, true);
24663+
v8::ScriptCompiler::Source source(source_text, origin);
24664+
Local<Module> module =
24665+
v8::ScriptCompiler::CompileModule(isolate, &source).ToLocalChecked();
24666+
module
24667+
->InstantiateModule(context,
24668+
ModuleEvaluateTerminateExecutionResolveCallback)
24669+
.ToChecked();
24670+
24671+
CHECK_EQ(module->GetStatus(), Module::kInstantiated);
24672+
return module;
24673+
}
24674+
24675+
void ModuleEvaluateTerminateExecution(
24676+
const v8::FunctionCallbackInfo<v8::Value>& args) {
24677+
v8::Isolate::GetCurrent()->TerminateExecution();
24678+
}
24679+
} // namespace
24680+
24681+
TEST(ModuleEvaluateTerminateExecution) {
24682+
LocalContext env;
24683+
v8::Isolate* isolate = env->GetIsolate();
24684+
v8::Isolate::Scope iscope(isolate);
24685+
v8::HandleScope scope(isolate);
24686+
v8::Local<v8::Context> context = v8::Context::New(isolate);
24687+
v8::Context::Scope cscope(context);
24688+
24689+
v8::Local<v8::Function> terminate_execution =
24690+
v8::Function::New(context, ModuleEvaluateTerminateExecution,
24691+
v8_str("terminate_execution"))
24692+
.ToLocalChecked();
24693+
context->Global()
24694+
->Set(context, v8_str("terminate_execution"), terminate_execution)
24695+
.FromJust();
24696+
24697+
Local<String> url = v8_str("www.test.com");
24698+
Local<String> source_text = v8_str(
24699+
"terminate_execution();"
24700+
"await Promise.resolve();");
24701+
v8::ScriptOrigin origin(isolate, url, 0, 0, false, -1, Local<v8::Value>(),
24702+
false, false, true);
24703+
v8::ScriptCompiler::Source source(source_text, origin);
24704+
Local<Module> module =
24705+
v8::ScriptCompiler::CompileModule(isolate, &source).ToLocalChecked();
24706+
module
24707+
->InstantiateModule(context,
24708+
ModuleEvaluateTerminateExecutionResolveCallback)
24709+
.ToChecked();
24710+
24711+
CHECK_EQ(module->GetStatus(), Module::kInstantiated);
24712+
TryCatch try_catch(isolate);
24713+
v8::MaybeLocal<Value> completion_value = module->Evaluate(context);
24714+
CHECK(completion_value.IsEmpty());
24715+
24716+
CHECK_EQ(module->GetStatus(), Module::kErrored);
24717+
CHECK(try_catch.HasCaught());
24718+
CHECK(try_catch.HasTerminated());
24719+
}
24720+
24721+
TEST(ModuleEvaluateImportTerminateExecution) {
24722+
LocalContext env;
24723+
v8::Isolate* isolate = env->GetIsolate();
24724+
v8::Isolate::Scope iscope(isolate);
24725+
v8::HandleScope scope(isolate);
24726+
v8::Local<v8::Context> context = v8::Context::New(isolate);
24727+
v8::Context::Scope cscope(context);
24728+
24729+
v8::Local<v8::Function> terminate_execution =
24730+
v8::Function::New(context, ModuleEvaluateTerminateExecution,
24731+
v8_str("terminate_execution"))
24732+
.ToLocalChecked();
24733+
context->Global()
24734+
->Set(context, v8_str("terminate_execution"), terminate_execution)
24735+
.FromJust();
24736+
24737+
Local<String> url = v8_str("www.test.com");
24738+
Local<String> source_text = v8_str(
24739+
"import './synthetic.module';"
24740+
"terminate_execution();"
24741+
"await Promise.resolve();");
24742+
v8::ScriptOrigin origin(isolate, url, 0, 0, false, -1, Local<v8::Value>(),
24743+
false, false, true);
24744+
v8::ScriptCompiler::Source source(source_text, origin);
24745+
Local<Module> module =
24746+
v8::ScriptCompiler::CompileModule(isolate, &source).ToLocalChecked();
24747+
module
24748+
->InstantiateModule(context,
24749+
ModuleEvaluateTerminateExecutionResolveCallback)
24750+
.ToChecked();
24751+
24752+
CHECK_EQ(module->GetStatus(), Module::kInstantiated);
24753+
TryCatch try_catch(isolate);
24754+
v8::MaybeLocal<Value> completion_value = module->Evaluate(context);
24755+
Local<v8::Promise> promise(
24756+
Local<v8::Promise>::Cast(completion_value.ToLocalChecked()));
24757+
CHECK_EQ(promise->State(), v8::Promise::kPending);
24758+
isolate->PerformMicrotaskCheckpoint();
24759+
24760+
// The exception thrown by terminate execution is not catchable by JavaScript
24761+
// so the promise can not be settled.
24762+
CHECK_EQ(promise->State(), v8::Promise::kPending);
24763+
CHECK_EQ(module->GetStatus(), Module::kEvaluated);
24764+
CHECK(try_catch.HasCaught());
24765+
CHECK(try_catch.HasTerminated());
24766+
}
24767+
2465224768
// Tests that the code cache does not confuse the same source code compiled as a
2465324769
// script and as a module.
2465424770
TEST(CodeCacheModuleScriptMismatch) {

0 commit comments

Comments
 (0)