Skip to content

Commit

Permalink
async_hooks: add HandleScopes to C++ embedder/addon API
Browse files Browse the repository at this point in the history
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: #24285
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
  • Loading branch information
addaleax authored and codebytere committed Jan 12, 2019
1 parent 39ccf14 commit fbf5321
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 0 deletions.
8 changes: 8 additions & 0 deletions src/async_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -694,13 +694,17 @@ MaybeLocal<Value> AsyncWrap::MakeCallback(const Local<Function> 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();
}


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();
Expand All @@ -711,6 +715,7 @@ async_context EmitAsyncInit(Isolate* isolate,
Local<Object> resource,
const char* name,
async_id trigger_async_id) {
v8::HandleScope handle_scope(isolate);
Local<String> type =
String::NewFromUtf8(isolate, name, v8::NewStringType::kInternalized)
.ToLocalChecked();
Expand All @@ -721,6 +726,7 @@ async_context EmitAsyncInit(Isolate* isolate,
Local<Object> resource,
v8::Local<v8::String> name,
async_id trigger_async_id) {
v8::HandleScope handle_scope(isolate);
Environment* env = Environment::GetCurrent(isolate);
CHECK_NOT_NULL(env);

Expand All @@ -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);
}
Expand Down
2 changes: 2 additions & 0 deletions test/addons/async-hello-world/binding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit fbf5321

Please sign in to comment.