From fcd7a7242890b30c3c143255234bdfe1855999b6 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 10 Nov 2018 16:35:38 +0100 Subject: [PATCH] async_hooks: add HandleScopes to C++ embedder/addon API Add `HandleScope`s to the public C++ API for embedders/addons, since these methods create V8 handles that should not leak into the outer scopes. In particular, for some of the methods it was not clear from the function signatures that these functions previously needed to be invoked with a `HandleScope`. PR-URL: https://github.com/nodejs/node/pull/24285 Reviewed-By: Gus Caplan Reviewed-By: James M Snell Reviewed-By: Joyee Cheung Reviewed-By: Michael Dawson --- src/async_wrap.cc | 8 ++++++++ test/addons/async-hello-world/binding.cc | 2 ++ 2 files changed, 10 insertions(+) diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 0738cd400997f1..a104d9a6557edf 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -694,6 +694,8 @@ MaybeLocal AsyncWrap::MakeCallback(const Local cb, async_id AsyncHooksGetExecutionAsyncId(Isolate* isolate) { + // Environment::GetCurrent() allocates a Local<> handle. + v8::HandleScope handle_scope(isolate); Environment* env = Environment::GetCurrent(isolate); if (env == nullptr) return -1; return env->execution_async_id(); @@ -701,6 +703,8 @@ async_id AsyncHooksGetExecutionAsyncId(Isolate* isolate) { async_id AsyncHooksGetTriggerAsyncId(Isolate* isolate) { + // Environment::GetCurrent() allocates a Local<> handle. + v8::HandleScope handle_scope(isolate); Environment* env = Environment::GetCurrent(isolate); if (env == nullptr) return -1; return env->trigger_async_id(); @@ -711,6 +715,7 @@ async_context EmitAsyncInit(Isolate* isolate, Local resource, const char* name, async_id trigger_async_id) { + v8::HandleScope handle_scope(isolate); Local type = String::NewFromUtf8(isolate, name, v8::NewStringType::kInternalized) .ToLocalChecked(); @@ -721,6 +726,7 @@ async_context EmitAsyncInit(Isolate* isolate, Local resource, v8::Local name, async_id trigger_async_id) { + v8::HandleScope handle_scope(isolate); Environment* env = Environment::GetCurrent(isolate); CHECK_NOT_NULL(env); @@ -741,6 +747,8 @@ async_context EmitAsyncInit(Isolate* isolate, } void EmitAsyncDestroy(Isolate* isolate, async_context asyncContext) { + // Environment::GetCurrent() allocates a Local<> handle. + v8::HandleScope handle_scope(isolate); AsyncWrap::EmitDestroy( Environment::GetCurrent(isolate), asyncContext.async_id); } diff --git a/test/addons/async-hello-world/binding.cc b/test/addons/async-hello-world/binding.cc index 1bf94ddd1d70bc..ff899628c4d56d 100644 --- a/test/addons/async-hello-world/binding.cc +++ b/test/addons/async-hello-world/binding.cc @@ -57,6 +57,8 @@ void AfterAsync(uv_work_t* r) { global, 2, argv).ToLocalChecked(); } + // None of the following operations should allocate handles into this scope. + v8::SealHandleScope seal_handle_scope(isolate); // cleanup node::EmitAsyncDestroy(isolate, req->context); req->callback.Reset();