1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

src: fix JSONParser leaking internal V8 scopes

Browse Source 
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>
This commit is contained in:
Keyhan Vakil 2023-11-14 08:20:53 -08:00 committed by GitHub
parent 36ecf8ca65
commit 09f4aa9489
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 18 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
src/json_parser.cc
View File

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

3
src/json_parser.h
View File

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