Skip to content

Commit

Permalink
deps: V8: cherry-pick ca5b0ec
Browse files Browse the repository at this point in the history
Original commit message:

[heap] Ensure SyntheticModule is initialized before next allocation

Ensure that all fields of `SyntheticModule` are set before creating
the exports hash table for it, because the latter may trigger
garbage collection, leading to crashes.

This has been causing failures in the Node.js CI over the last weeks,
after making the creating of synthetic modules part of Node’s
startup sequence.

(I am generally not very familiar with this part of the V8
code and there might be a better way, or possibly a way to add a
reliable regression test, that I am not aware of.)

Refs: #30498
Refs: #30648
Change-Id: I32da4b7bd888c6ec1421f34f5bd52e7bad154c1e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1939752
Commit-Queue: Ulan Degenbaev <[email protected]>
Reviewed-by: Ulan Degenbaev <[email protected]>
Cr-Commit-Position: refs/heads/master@{#65247}

Refs: https://github.com/v8/v8/commit/ \
ca5b0ec2722d2af4551c01ca78921fa16a26ae72
Fixes: #30498
Fixes: #30648

PR-URL: #30708
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
  • Loading branch information
addaleax authored and gireeshpunathil committed Nov 29, 2019
1 parent 99d1f6f commit 29b5432
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 5 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,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.20',
'v8_embedder_string': '-node.21',

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

Expand Down
10 changes: 6 additions & 4 deletions deps/v8/src/heap/factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3070,20 +3070,22 @@ Handle<SyntheticModule> Factory::NewSyntheticModule(
Handle<String> module_name, Handle<FixedArray> export_names,
v8::Module::SyntheticModuleEvaluationSteps evaluation_steps) {
ReadOnlyRoots roots(isolate());
Handle<SyntheticModule> module(
SyntheticModule::cast(New(synthetic_module_map(), AllocationType::kOld)),
isolate());

Handle<ObjectHashTable> exports =
ObjectHashTable::New(isolate(), static_cast<int>(export_names->length()));
Handle<Foreign> evaluation_steps_foreign =
NewForeign(reinterpret_cast<i::Address>(evaluation_steps));
module->set_exports(*exports);

Handle<SyntheticModule> module(
SyntheticModule::cast(New(synthetic_module_map(), AllocationType::kOld)),
isolate());
module->set_hash(isolate()->GenerateIdentityHash(Smi::kMaxValue));
module->set_module_namespace(roots.undefined_value());
module->set_status(Module::kUninstantiated);
module->set_exception(roots.the_hole_value());
module->set_name(*module_name);
module->set_export_names(*export_names);
module->set_exports(*exports);
module->set_evaluation_steps(*evaluation_steps_foreign);
return module;
}
Expand Down
25 changes: 25 additions & 0 deletions deps/v8/test/cctest/test-api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23918,6 +23918,31 @@ TEST(CreateSyntheticModule) {
CHECK_EQ(i_module->status(), i::Module::kInstantiated);
}

TEST(CreateSyntheticModuleGC) {
// Try to make sure that CreateSyntheticModule() deals well with a GC
// happening during its execution.
i::FLAG_gc_interval = 10;
i::FLAG_inline_new = false;

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);

std::vector<v8::Local<v8::String>> export_names{v8_str("default")};
v8::Local<v8::String> module_name =
v8_str("CreateSyntheticModule-TestSyntheticModuleGC");

for (int i = 0; i < 200; i++) {
Local<Module> module = v8::Module::CreateSyntheticModule(
isolate, module_name, export_names,
UnexpectedSyntheticModuleEvaluationStepsCallback);
USE(module);
}
}

TEST(SyntheticModuleSetExports) {
LocalContext env;
v8::Isolate* isolate = env->GetIsolate();
Expand Down

0 comments on commit 29b5432

Please sign in to comment.