src: add IsolateScopes before using isolates

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.

This also introduces RAIIIsolateWithoutEntering which is used in
JSONParser to avoid otherwise exposing the Isolate::Scope outside of the
class.

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
PR-URL: #50680
Refs: v8/v8@475c8cd
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: James M Snell <[email protected]>

src/api/environment.cc

@@ -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);
 }
@@ -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.
@@ -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();

src/env.cc

@@ -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)) {
@@ -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)                                           \

src/json_parser.cc

@@ -17,6 +17,7 @@ bool JSONParser::Parse(const std::string& content) {
   DCHECK(!parsed_);
 
   Isolate* isolate = isolate_.get();
+  v8::Isolate::Scope isolate_scope(isolate);
   v8::HandleScope handle_scope(isolate);
 
   Local<Context> context = Context::New(isolate);
@@ -45,6 +46,7 @@ bool JSONParser::Parse(const std::string& content) {
 std::optional<std::string> JSONParser::GetTopLevelStringField(
     std::string_view field) {
   Isolate* isolate = isolate_.get();
+  v8::Isolate::Scope isolate_scope(isolate);
   v8::HandleScope handle_scope(isolate);
 
   Local<Context> context = context_.Get(isolate);
@@ -70,6 +72,7 @@ std::optional<std::string> JSONParser::GetTopLevelStringField(
 
 std::optional<bool> JSONParser::GetTopLevelBoolField(std::string_view field) {
   Isolate* isolate = isolate_.get();
+  v8::Isolate::Scope isolate_scope(isolate);
   v8::HandleScope handle_scope(isolate);
 
   Local<Context> context = context_.Get(isolate);

src/json_parser.h

@@ -24,7 +24,7 @@ class JSONParser {
  private:
   // We might want a lighter-weight JSON parser for this use case. But for now
   // using V8 is good enough.
-  RAIIIsolate isolate_;
+  RAIIIsolateWithoutEntering isolate_;
 
   v8::Global<v8::Context> context_;
   v8::Global<v8::Object> content_;

src/node_sea.cc

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

src/util.cc

@@ -678,7 +678,7 @@ Local<String> UnionBytes::ToStringChecked(Isolate* isolate) const {
   }
 }
 
-RAIIIsolate::RAIIIsolate(const SnapshotData* data)
+RAIIIsolateWithoutEntering::RAIIIsolateWithoutEntering(const SnapshotData* data)
     : allocator_{ArrayBuffer::Allocator::NewDefaultAllocator()} {
   isolate_ = Isolate::Allocate();
   CHECK_NOT_NULL(isolate_);
@@ -692,9 +692,14 @@ RAIIIsolate::RAIIIsolate(const SnapshotData* data)
   Isolate::Initialize(isolate_, params);
 }
 
-RAIIIsolate::~RAIIIsolate() {
+RAIIIsolateWithoutEntering::~RAIIIsolateWithoutEntering() {
   per_process::v8_platform.Platform()->UnregisterIsolate(isolate_);
   isolate_->Dispose();
 }
 
+RAIIIsolate::RAIIIsolate(const SnapshotData* data)
+    : isolate_{data}, isolate_scope_{isolate_.get()} {}
+
+RAIIIsolate::~RAIIIsolate() {}
+
 }  // namespace node

src/util.h

@@ -969,11 +969,11 @@ void SetConstructorFunction(v8::Isolate* isolate,
                             SetConstructorFunctionFlag flag =
                                 SetConstructorFunctionFlag::SET_CLASS_NAME);
 
-// Simple RAII class to spin up a v8::Isolate instance.
-class RAIIIsolate {
+// Like RAIIIsolate, except doesn't enter the isolate while it's in scope.
+class RAIIIsolateWithoutEntering {
  public:
-  explicit RAIIIsolate(const SnapshotData* data = nullptr);
-  ~RAIIIsolate();
+  explicit RAIIIsolateWithoutEntering(const SnapshotData* data = nullptr);
+  ~RAIIIsolateWithoutEntering();
 
   v8::Isolate* get() const { return isolate_; }
 
@@ -982,6 +982,20 @@ class RAIIIsolate {
   v8::Isolate* isolate_;
 };
 
+// Simple RAII class to spin up a v8::Isolate instance and enter it
+// immediately.
+class RAIIIsolate {
+ public:
+  explicit RAIIIsolate(const SnapshotData* data = nullptr);
+  ~RAIIIsolate();
+
+  v8::Isolate* get() const { return isolate_.get(); }
+
+ private:
+  RAIIIsolateWithoutEntering isolate_;
+  v8::Isolate::Scope isolate_scope_;
+};
+
 }  // namespace node
 
 #endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS