src: fix JSONParser leaking internal V8 scopes

JSONParser uses V8's JSON.parse (for now), meaning that its uses handles and contexts. JSONParser was leaking its internal HandleScope and Context::Scope. Move the scope construction to the member functions to prevent those scopes from leaking. Refs: nodejs#50680 (comment) PR-URL: nodejs#50688 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
martenrichter · Nov 26, 2023 · e9c6d17 · e9c6d17
1 parent 032d766
commit e9c6d17
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 7 deletions.
diff --git a/src/json_parser.cc b/src/json_parser.cc
@@ -11,16 +11,16 @@ using v8::Object;
 using v8::String;
 using v8::Value;
 
-JSONParser::JSONParser()
-    : handle_scope_(isolate_.get()),
-      context_(isolate_.get(), Context::New(isolate_.get())),
-      context_scope_(context_.Get(isolate_.get())) {}
+JSONParser::JSONParser() {}
 
 bool JSONParser::Parse(const std::string& content) {
   DCHECK(!parsed_);
 
   Isolate* isolate = isolate_.get();
-  Local<Context> context = context_.Get(isolate);
+  v8::HandleScope handle_scope(isolate);
+
+  Local<Context> context = Context::New(isolate);
+  Context::Scope context_scope(context);
 
   // It's not a real script, so don't print the source line.
   errors::PrinterTryCatch bootstrapCatch(
@@ -34,16 +34,24 @@ bool JSONParser::Parse(const std::string& content) {
       !result_value->IsObject()) {
     return false;
   }
+
+  context_.Reset(isolate, context);
   content_.Reset(isolate, result_value.As<Object>());
   parsed_ = true;
+
   return true;
 }
 
 std::optional<std::string> JSONParser::GetTopLevelStringField(
     std::string_view field) {
   Isolate* isolate = isolate_.get();
+  v8::HandleScope handle_scope(isolate);
+
   Local<Context> context = context_.Get(isolate);
+  Context::Scope context_scope(context);
+
   Local<Object> content_object = content_.Get(isolate);
+
   Local<Value> value;
   // It's not a real script, so don't print the source line.
   errors::PrinterTryCatch bootstrapCatch(
@@ -62,7 +70,11 @@ std::optional<std::string> JSONParser::GetTopLevelStringField(
 
 std::optional<bool> JSONParser::GetTopLevelBoolField(std::string_view field) {
   Isolate* isolate = isolate_.get();
+  v8::HandleScope handle_scope(isolate);
+
   Local<Context> context = context_.Get(isolate);
+  Context::Scope context_scope(context);
+
   Local<Object> content_object = content_.Get(isolate);
   Local<Value> value;
   bool has_field;

diff --git a/src/json_parser.h b/src/json_parser.h
@@ -25,9 +25,8 @@ 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::HandleScope handle_scope_;
+
   v8::Global<v8::Context> context_;
-  v8::Context::Scope context_scope_;
   v8::Global<v8::Object> content_;
   bool parsed_ = false;
 };