From 961b3547bdcb6f490da5d60a3f8abb45cf6f40f6 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 13 Jul 2016 16:06:18 +0200 Subject: [PATCH 1/4] src: fix handle leak in Buffer::New() Fix handle leaks in Buffer::New() and Buffer::Copy() by creating the handle scope before looking up the env with Environment::GetCurrent(). Environment::GetCurrent() calls v8::Isolate::GetCurrentContext(), which creates a handle in the current scope, i.e., the scope created by the caller of Buffer::New() or Buffer::Copy(). PR-URL: https://github.com/nodejs/node/pull/7711 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Trevor Norris --- src/node_buffer.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/node_buffer.cc b/src/node_buffer.cc index d07632a101d491..dfdf68b9ce5584 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -298,8 +298,8 @@ MaybeLocal New(Environment* env, size_t length) { MaybeLocal Copy(Isolate* isolate, const char* data, size_t length) { + EscapableHandleScope handle_scope(isolate); Environment* env = Environment::GetCurrent(isolate); - EscapableHandleScope handle_scope(env->isolate()); Local obj; if (Buffer::Copy(env, data, length).ToLocal(&obj)) return handle_scope.Escape(obj); @@ -348,8 +348,8 @@ MaybeLocal 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 obj; if (Buffer::New(env, data, length, callback, hint).ToLocal(&obj)) return handle_scope.Escape(obj); @@ -382,8 +382,8 @@ MaybeLocal New(Environment* env, MaybeLocal New(Isolate* isolate, char* data, size_t length) { + EscapableHandleScope handle_scope(isolate); Environment* env = Environment::GetCurrent(isolate); - EscapableHandleScope handle_scope(env->isolate()); Local obj; if (Buffer::New(env, data, length).ToLocal(&obj)) return handle_scope.Escape(obj); From 11f1da8144fb500e2a6f92a71800d86d76ffd614 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 13 Jul 2016 16:16:40 +0200 Subject: [PATCH 2/4] src: fix handle leak in BuildStatsObject() Create a handle scope before performing a check that creates a handle, otherwise the handle is leaked into the handle scope of the caller. PR-URL: https://github.com/nodejs/node/pull/7711 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Trevor Norris --- src/node_file.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/node_file.cc b/src/node_file.cc index fadf4cef93cd61..7aa3faa2e0f82a 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -344,11 +344,11 @@ static void Close(const FunctionCallbackInfo& args) { Local 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... From d6cac6dfea00eb3a4b281592c6b283d446b59d51 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 13 Jul 2016 16:16:52 +0200 Subject: [PATCH 3/4] src: fix handle leak in UDPWrap::Instantiate() Create a handle scope before performing a check that creates a handle, otherwise the handle is leaked into the handle scope of the caller. PR-URL: https://github.com/nodejs/node/pull/7711 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Trevor Norris --- src/udp_wrap.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index e395ee85e0852e..e0f51cbf589699 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -394,9 +394,9 @@ void UDPWrap::OnRecv(uv_udp_t* handle, Local 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 ptr = External::New(env->isolate(), parent); return scope.Escape(env->udp_constructor_function()->NewInstance(1, &ptr)); } From 6f3ac5d6b400e8bf17963da76640b9ec143dd449 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Wed, 13 Jul 2016 16:38:27 +0200 Subject: [PATCH 4/4] src: remove unnecessary HandleScopes API function callbacks run inside an implicit HandleScope. We don't need to explicitly create one and in fact introduce some unnecessary overhead when we do. PR-URL: https://github.com/nodejs/node/pull/7711 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Trevor Norris --- src/node_crypto.cc | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 7ce79fda7008d3..e047b30443df4f 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -1776,8 +1776,6 @@ template void SSLWrap::SetOCSPResponse( const v8::FunctionCallbackInfo& args) { #ifdef NODE__HAVE_TLSEXT_STATUS_CB - HandleScope scope(args.GetIsolate()); - Base* w = Unwrap(args.Holder()); if (args.Length() < 1 || !Buffer::HasInstance(args[0])) return w->env()->ThrowTypeError("Must give a Buffer as first argument"); @@ -1791,8 +1789,6 @@ template void SSLWrap::RequestOCSP( const v8::FunctionCallbackInfo& args) { #ifdef NODE__HAVE_TLSEXT_STATUS_CB - HandleScope scope(args.GetIsolate()); - Base* w = Unwrap(args.Holder()); SSL_set_tlsext_status_type(w->ssl_, TLSEXT_STATUSTYPE_ocsp); @@ -1804,7 +1800,6 @@ void SSLWrap::RequestOCSP( template void SSLWrap::SetMaxSendFragment( const v8::FunctionCallbackInfo& args) { - HandleScope scope(args.GetIsolate()); CHECK(args.Length() >= 1 && args[0]->IsNumber()); Base* w = Unwrap(args.Holder());