Skip to content

Commit

Permalink
src: add IsolateScopes before using isolates
Browse files Browse the repository at this point in the history
The V8 API requires entering an isolate before using it. We were often
not doing this, which worked fine in practice. However when (multi-cage)
pointer compression is enabled, the correct isolate needs to be active
in order to decompress pointers correctly, otherwise it causes crashes.

Fix this by sprinkling in some calls to v8::Isolate::Scope::Scope where
they were missing.

Tested by compiling with `--experimental-enable-pointer-compression`
locally and running all tests.

Refs: nodejs/build#3204 (comment)
Refs: https://bugs.chromium.org/p/v8/issues/detail?id=14292
  • Loading branch information
kvakil committed Nov 12, 2023
1 parent d52c5bc commit 9dc6d7c
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 1 deletion.
7 changes: 7 additions & 0 deletions src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,8 @@ void SetIsolateMiscHandlers(v8::Isolate* isolate, const IsolateSettings& s) {

void SetIsolateUpForNode(v8::Isolate* isolate,
const IsolateSettings& settings) {
Isolate::Scope isolate_scope(isolate);

SetIsolateErrorHandlers(isolate, settings);
SetIsolateMiscHandlers(isolate, settings);
}
Expand Down Expand Up @@ -354,6 +356,9 @@ Isolate* NewIsolate(Isolate::CreateParams* params,

SetIsolateCreateParamsForNode(params);
Isolate::Initialize(isolate, *params);

Isolate::Scope isolate_scope(isolate);

if (snapshot_data == nullptr) {
// If in deserialize mode, delay until after the deserialization is
// complete.
Expand Down Expand Up @@ -428,6 +433,8 @@ Environment* CreateEnvironment(
ThreadId thread_id,
std::unique_ptr<InspectorParentHandle> inspector_parent_handle) {
Isolate* isolate = isolate_data->isolate();

Isolate::Scope isolate_scope(isolate);
HandleScope handle_scope(isolate);

const bool use_snapshot = context.IsEmpty();
Expand Down
3 changes: 3 additions & 0 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,8 @@ IsolateDataSerializeInfo IsolateData::Serialize(SnapshotCreator* creator) {

void IsolateData::DeserializeProperties(const IsolateDataSerializeInfo* info) {
size_t i = 0;

v8::Isolate::Scope isolate_scope(isolate_);
HandleScope handle_scope(isolate_);

if (per_process::enabled_debug_list.enabled(DebugCategory::MKSNAPSHOT)) {
Expand Down Expand Up @@ -431,6 +433,7 @@ void IsolateData::CreateProperties() {
// One byte because our strings are ASCII and we can safely skip V8's UTF-8
// decoding step.

v8::Isolate::Scope isolate_scope(isolate_);
HandleScope handle_scope(isolate_);

#define V(PropertyName, StringValue) \
Expand Down
3 changes: 2 additions & 1 deletion src/json_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ using v8::String;
using v8::Value;

JSONParser::JSONParser()
: handle_scope_(isolate_.get()),
: isolate_scope_(isolate_.get()),
handle_scope_(isolate_.get()),
context_(isolate_.get(), Context::New(isolate_.get())),
context_scope_(context_.Get(isolate_.get())) {}

Expand Down
3 changes: 3 additions & 0 deletions src/json_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ class JSONParser {
// We might want a lighter-weight JSON parser for this use case. But for now
// using V8 is good enough.
RAIIIsolate isolate_;

v8::Isolate::Scope isolate_scope_;
v8::HandleScope handle_scope_;

v8::Global<v8::Context> context_;
v8::Context::Scope context_scope_;
v8::Global<v8::Object> content_;
Expand Down
2 changes: 2 additions & 0 deletions src/node_sea.cc
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,9 @@ std::optional<std::string> GenerateCodeCache(std::string_view main_path,
RAIIIsolate raii_isolate(SnapshotBuilder::GetEmbeddedSnapshotData());
Isolate* isolate = raii_isolate.get();

v8::Isolate::Scope isolate_scope(isolate);
HandleScope handle_scope(isolate);

Local<Context> context = Context::New(isolate);
Context::Scope context_scope(context);

Expand Down

0 comments on commit 9dc6d7c

Please sign in to comment.