Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: fix handle leaks #7711

Merged
merged 4 commits into from
Jul 18, 2016
Merged

src: fix handle leaks #7711

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6754bae
src: fix handle leak in Buffer::New()
bnoordhuis Jul 13, 2016
c354b24
src: fix handle leak in BuildStatsObject()
bnoordhuis Jul 13, 2016
4e989b0
src: fix handle leak in UDPWrap::Instantiate()
bnoordhuis Jul 13, 2016
48c52d7
src: remove unnecessary HandleScopes
bnoordhuis Jul 13, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/node_buffer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,8 @@ MaybeLocal<Object> New(Environment* env, size_t length) {


MaybeLocal<Object> Copy(Isolate* isolate, const char* data, size_t length) {
EscapableHandleScope handle_scope(isolate);
Environment* env = Environment::GetCurrent(isolate);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain how this is leaky? Just out of interest.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Environment::GetCurrent() calls Isolate::GetCurrentContext() which creates a Local<Context> in the current handle scope, i.e., the scope of the caller.

EscapableHandleScope handle_scope(env->isolate());
Local<Object> obj;
if (Buffer::Copy(env, data, length).ToLocal(&obj))
return handle_scope.Escape(obj);
Expand Down Expand Up @@ -395,8 +395,8 @@ MaybeLocal<Object> New(Isolate* isolate,
size_t length,
FreeCallback callback,
void* hint) {
EscapableHandleScope handle_scope(isolate);
Environment* env = Environment::GetCurrent(isolate);
EscapableHandleScope handle_scope(env->isolate());
Local<Object> obj;
if (Buffer::New(env, data, length, callback, hint).ToLocal(&obj))
return handle_scope.Escape(obj);
Expand Down Expand Up @@ -434,8 +434,8 @@ MaybeLocal<Object> New(Environment* env,


MaybeLocal<Object> New(Isolate* isolate, char* data, size_t length) {
EscapableHandleScope handle_scope(isolate);
Environment* env = Environment::GetCurrent(isolate);
EscapableHandleScope handle_scope(env->isolate());
Local<Object> obj;
if (Buffer::New(env, data, length).ToLocal(&obj))
return handle_scope.Escape(obj);
Expand Down
7 changes: 0 additions & 7 deletions src/node_crypto.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1822,8 +1822,6 @@ template <class Base>
void SSLWrap<Base>::SetOCSPResponse(
const v8::FunctionCallbackInfo<v8::Value>& args) {
#ifdef NODE__HAVE_TLSEXT_STATUS_CB
HandleScope scope(args.GetIsolate());

Base* w;
ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder());
Environment* env = w->env();
Expand All @@ -1842,8 +1840,6 @@ template <class Base>
void SSLWrap<Base>::RequestOCSP(
const v8::FunctionCallbackInfo<v8::Value>& args) {
#ifdef NODE__HAVE_TLSEXT_STATUS_CB
HandleScope scope(args.GetIsolate());

Base* w;
ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder());

Expand Down Expand Up @@ -1901,7 +1897,6 @@ void SSLWrap<Base>::GetEphemeralKeyInfo(
template <class Base>
void SSLWrap<Base>::SetMaxSendFragment(
const v8::FunctionCallbackInfo<v8::Value>& args) {
HandleScope scope(args.GetIsolate());
CHECK(args.Length() >= 1 && args[0]->IsNumber());

Base* w;
Expand Down Expand Up @@ -2204,7 +2199,6 @@ template <class Base>
void SSLWrap<Base>::GetALPNNegotiatedProto(
const FunctionCallbackInfo<v8::Value>& args) {
#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation
HandleScope scope(args.GetIsolate());
Base* w;
ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder());

Expand All @@ -2226,7 +2220,6 @@ template <class Base>
void SSLWrap<Base>::SetALPNProtocols(
const FunctionCallbackInfo<v8::Value>& args) {
#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation
HandleScope scope(args.GetIsolate());
Base* w;
ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder());
Environment* env = w->env();
Expand Down
4 changes: 2 additions & 2 deletions src/node_file.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,11 +420,11 @@ static void Close(const FunctionCallbackInfo<Value>& args) {


Local<Value> BuildStatsObject(Environment* env, const uv_stat_t* s) {
EscapableHandleScope handle_scope(env->isolate());

// If you hit this assertion, you forgot to enter the v8::Context first.
CHECK_EQ(env->context(), env->isolate()->GetCurrentContext());

EscapableHandleScope handle_scope(env->isolate());

// The code below is very nasty-looking but it prevents a segmentation fault
// when people run JS code like the snippet below. It's apparently more
// common than you would expect, several people have reported this crash...
Expand Down
2 changes: 1 addition & 1 deletion src/udp_wrap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,9 +436,9 @@ void UDPWrap::OnRecv(uv_udp_t* handle,


Local<Object> UDPWrap::Instantiate(Environment* env, AsyncWrap* parent) {
EscapableHandleScope scope(env->isolate());
// If this assert fires then Initialize hasn't been called yet.
CHECK_EQ(env->udp_constructor_function().IsEmpty(), false);
EscapableHandleScope scope(env->isolate());
Local<Value> ptr = External::New(env->isolate(), parent);
return scope.Escape(env->udp_constructor_function()
->NewInstance(env->context(), 1, &ptr).ToLocalChecked());
Expand Down