From 2e8e237ce21f05519b3a964ccb24a8382ccc2f02 Mon Sep 17 00:00:00 2001
From: Keyhan Vakil <kvakil@sylph.kvakil.me>
Date: Tue, 14 Nov 2023 08:20:53 -0800
Subject: [PATCH] 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: https://github.com/nodejs/node/pull/50680#discussion_r1390363367
PR-URL: https://github.com/nodejs/node/pull/50688
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
---
 src/json_parser.cc | 22 +++++++++++++++++-----
 src/json_parser.h  |  3 +--
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/src/json_parser.cc b/src/json_parser.cc
index 1e19e174833fa5..7b405cfc3b331e 100644
--- 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
index f2e7d592d9aaa7..a4c5b3e96dd23d 100644
--- 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;
 };