Skip to content

Commit

Permalink
src: fix JSONParser leaking internal V8 scopes
Browse files Browse the repository at this point in the history
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: #50680 (comment)
PR-URL: #50688
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
  • Loading branch information
kvakil authored and UlisesGascon committed Dec 19, 2023
1 parent 3d82d38 commit 436c3ae
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 7 deletions.
22 changes: 17 additions & 5 deletions src/json_parser.cc
Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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;
Expand Down
3 changes: 1 addition & 2 deletions src/json_parser.h
Expand Up @@ -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;
};
Expand Down

0 comments on commit 436c3ae

Please sign in to comment.