Skip to content

[v8.x] async_hooks: C++ Embedder API overhaul #14109

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

Closed
addaleax wants to merge 3 commits into from
Closed

[v8.x] async_hooks: C++ Embedder API overhaul #14109

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3685f04
async_hooks: C++ Embedder API overhaul
AndreasMadsen Jul 6, 2017
85597ca
[squash] ABI/API compat
addaleax Jul 6, 2017
a2f61c4
[squash] add deprecation warnings for deprecated MakeCallbacks
addaleax Jul 7, 2017
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
Next Next commit
async_hooks: C++ Embedder API overhaul 
* Fix AsyncHooksGetTriggerAsyncId such it corresponds to
async_hooks.triggerAsyncId and not async_hooks.initTriggerId.
* Use an async_context struct instead of two async_uid values.
  This change was necessary since the fixing
  AsyncHooksGetTriggerAsyncId otherwise makes it impossible to
  get the correct default trigger id. It also prevents an invalid
  triggerAsyncId in MakeCallback.
* Rename async_uid to async_id for consistency
* Rename get_uid to get_async_id
* Add get_trigger_async_id to AsyncResource class

PR-URL: #14040
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
@AndreasMadsen @addaleax
AndreasMadsen authored and addaleax committed Jul 11, 2017
commit 3685f0447da42afdf42550f6d2171b7283f9d794
39 changes: 25 additions & 14 deletions src/async-wrap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -741,40 +741,51 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
/* Public C++ embedder API */


async_uid AsyncHooksGetExecutionAsyncId(Isolate* isolate) {
async_id AsyncHooksGetExecutionAsyncId(Isolate* isolate) {
return Environment::GetCurrent(isolate)->current_async_id();
}

async_uid AsyncHooksGetCurrentId(Isolate* isolate) {
async_id AsyncHooksGetCurrentId(Isolate* isolate) {
return AsyncHooksGetExecutionAsyncId(isolate);
}


async_uid AsyncHooksGetTriggerAsyncId(Isolate* isolate) {
return Environment::GetCurrent(isolate)->get_init_trigger_id();
async_id AsyncHooksGetTriggerAsyncId(Isolate* isolate) {
return Environment::GetCurrent(isolate)->trigger_id();
}

async_uid AsyncHooksGetTriggerId(Isolate* isolate) {
async_id AsyncHooksGetTriggerId(Isolate* isolate) {
return AsyncHooksGetTriggerAsyncId(isolate);
}


async_uid EmitAsyncInit(Isolate* isolate,
Local<Object> resource,
const char* name,
async_uid trigger_id) {
async_context EmitAsyncInit(Isolate* isolate,
Local<Object> resource,
const char* name,
async_id trigger_async_id) {
Environment* env = Environment::GetCurrent(isolate);
async_uid async_id = env->new_async_id();

// Initialize async context struct
if (trigger_async_id == -1)
trigger_async_id = env->get_init_trigger_id();

async_context context = {
env->new_async_id(), // async_id_
trigger_async_id // trigger_async_id_
};

// Run init hooks
Local<String> type =
String::NewFromUtf8(isolate, name, v8::NewStringType::kInternalized)
.ToLocalChecked();
AsyncWrap::EmitAsyncInit(env, resource, type, async_id, trigger_id);
return async_id;
AsyncWrap::EmitAsyncInit(env, resource, type, context.async_id,
context.trigger_async_id);

return context;
}

void EmitAsyncDestroy(Isolate* isolate, async_uid id) {
PushBackDestroyId(Environment::GetCurrent(isolate), id);
void EmitAsyncDestroy(Isolate* isolate, async_context asyncContext) {
PushBackDestroyId(Environment::GetCurrent(isolate), asyncContext.async_id);
}

} // namespace node
Expand Down
2 changes: 1 addition & 1 deletion src/inspector_agent.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@ bool Agent::StartIoThread(bool wait_for_connect) {
message
};
MakeCallback(parent_env_->isolate(), process_object, emit_fn.As<Function>(),
arraysize(argv), argv, 0, 0);
arraysize(argv), argv, {0, 0});

return true;
}
Expand Down
49 changes: 23 additions & 26 deletions src/node.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ static void CheckImmediate(uv_check_t* handle) {
env->immediate_callback_string(),
0,
nullptr,
0, 0).ToLocalChecked();
{0, 0}).ToLocalChecked();
}


Expand Down Expand Up @@ -1298,8 +1298,7 @@ MaybeLocal<Value> MakeCallback(Environment* env,
const Local<Function> callback,
int argc,
Local<Value> argv[],
double async_id,
double trigger_id) {
async_context asyncContext) {
// If you hit this assertion, you forgot to enter the v8::Context first.
CHECK_EQ(env->context(), env->isolate()->GetCurrentContext());

Expand All @@ -1321,10 +1320,12 @@ MaybeLocal<Value> MakeCallback(Environment* env,
MaybeLocal<Value> ret;

{
AsyncHooks::ExecScope exec_scope(env, async_id, trigger_id);
AsyncHooks::ExecScope exec_scope(env, asyncContext.async_id,
asyncContext.trigger_async_id);

if (async_id != 0) {
if (!AsyncWrap::EmitBefore(env, async_id)) return Local<Value>();
if (asyncContext.async_id != 0) {
if (!AsyncWrap::EmitBefore(env, asyncContext.async_id))
return Local<Value>();
}

ret = callback->Call(env->context(), recv, argc, argv);
Expand All @@ -1336,8 +1337,9 @@ MaybeLocal<Value> MakeCallback(Environment* env,
ret : Undefined(env->isolate());
}

if (async_id != 0) {
if (!AsyncWrap::EmitAfter(env, async_id)) return Local<Value>();
if (asyncContext.async_id != 0) {
if (!AsyncWrap::EmitAfter(env, asyncContext.async_id))
return Local<Value>();
}
}

Expand All @@ -1358,8 +1360,8 @@ MaybeLocal<Value> MakeCallback(Environment* env,

// Make sure the stack unwound properly. If there are nested MakeCallback's
// then it should return early and not reach this code.
CHECK_EQ(env->current_async_id(), async_id);
CHECK_EQ(env->trigger_id(), trigger_id);
CHECK_EQ(env->current_async_id(), asyncContext.async_id);
CHECK_EQ(env->trigger_id(), asyncContext.trigger_async_id);

Local<Object> process = env->process_object();

Expand All @@ -1384,13 +1386,11 @@ MaybeLocal<Value> MakeCallback(Isolate* isolate,
const char* method,
int argc,
Local<Value> argv[],
async_uid async_id,
async_uid trigger_id) {
async_context asyncContext) {
Local<String> method_string =
String::NewFromUtf8(isolate, method, v8::NewStringType::kNormal)
.ToLocalChecked();
return MakeCallback(isolate, recv, method_string, argc, argv,
async_id, trigger_id);
return MakeCallback(isolate, recv, method_string, argc, argv, asyncContext);
}


Expand All @@ -1399,14 +1399,12 @@ MaybeLocal<Value> MakeCallback(Isolate* isolate,
Local<String> symbol,
int argc,
Local<Value> argv[],
async_uid async_id,
async_uid trigger_id) {
async_context asyncContext) {
Local<Value> callback_v = recv->Get(symbol);
if (callback_v.IsEmpty()) return Local<Value>();
if (!callback_v->IsFunction()) return Local<Value>();
Local<Function> callback = callback_v.As<Function>();
return MakeCallback(isolate, recv, callback, argc, argv,
async_id, trigger_id);
return MakeCallback(isolate, recv, callback, argc, argv, asyncContext);
}


Expand All @@ -1415,8 +1413,7 @@ MaybeLocal<Value> MakeCallback(Isolate* isolate,
Local<Function> callback,
int argc,
Local<Value> argv[],
async_uid async_id,
async_uid trigger_id) {
async_context asyncContext) {
// Observe the following two subtleties:
//
// 1. The environment is retrieved from the callback function's context.
Expand All @@ -1427,7 +1424,7 @@ MaybeLocal<Value> MakeCallback(Isolate* isolate,
Environment* env = Environment::GetCurrent(callback->CreationContext());
Context::Scope context_scope(env->context());
return MakeCallback(env, recv.As<Value>(), callback, argc, argv,
async_id, trigger_id);
asyncContext);
}


Expand All @@ -1440,7 +1437,7 @@ Local<Value> MakeCallback(Isolate* isolate,
Local<Value>* argv) {
EscapableHandleScope handle_scope(isolate);
return handle_scope.Escape(
MakeCallback(isolate, recv, method, argc, argv, 0, 0)
MakeCallback(isolate, recv, method, argc, argv, {0, 0})
.FromMaybe(Local<Value>()));
}

Expand All @@ -1452,7 +1449,7 @@ Local<Value> MakeCallback(Isolate* isolate,
Local<Value>* argv) {
EscapableHandleScope handle_scope(isolate);
return handle_scope.Escape(
MakeCallback(isolate, recv, symbol, argc, argv, 0, 0)
MakeCallback(isolate, recv, symbol, argc, argv, {0, 0})
.FromMaybe(Local<Value>()));
}

Expand All @@ -1464,7 +1461,7 @@ Local<Value> MakeCallback(Isolate* isolate,
Local<Value>* argv) {
EscapableHandleScope handle_scope(isolate);
return handle_scope.Escape(
MakeCallback(isolate, recv, callback, argc, argv, 0, 0)
MakeCallback(isolate, recv, callback, argc, argv, {0, 0})
.FromMaybe(Local<Value>()));
}

Expand Down Expand Up @@ -4448,7 +4445,7 @@ void EmitBeforeExit(Environment* env) {
};
MakeCallback(env->isolate(),
process_object, "emit", arraysize(args), args,
0, 0).ToLocalChecked();
{0, 0}).ToLocalChecked();
}


Expand All @@ -4469,7 +4466,7 @@ int EmitExit(Environment* env) {

MakeCallback(env->isolate(),
process_object, "emit", arraysize(args), args,
0, 0).ToLocalChecked();
{0, 0}).ToLocalChecked();

// Reload exit code, it may be changed by `emit('exit')`
return process_object->Get(exitCode)->Int32Value();
Expand Down
72 changes: 40 additions & 32 deletions src/node.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ inline v8::Local<v8::Value> UVException(int errorno,
* These methods need to be called in a HandleScope.
*
* It is preferred that you use the `MakeCallback` overloads taking
* `async_uid` arguments.
* `async_id` arguments.
*/

NODE_EXTERN v8::Local<v8::Value> MakeCallback(
Expand Down Expand Up @@ -517,7 +517,11 @@ typedef void (*promise_hook_func) (v8::PromiseHookType type,
v8::Local<v8::Value> parent,
void* arg);

typedef double async_uid;
typedef double async_id;
struct async_context {
::node::async_id async_id;
::node::async_id trigger_async_id;
};

/* Registers an additional v8::PromiseHook wrapper. This API exists because V8
* itself supports only a single PromiseHook. */
Expand All @@ -528,17 +532,17 @@ NODE_EXTERN void AddPromiseHook(v8::Isolate* isolate,
/* Returns the id of the current execution context. If the return value is
* zero then no execution has been set. This will happen if the user handles
* I/O from native code. */
NODE_EXTERN async_uid AsyncHooksGetExecutionAsyncId(v8::Isolate* isolate);
NODE_EXTERN async_id AsyncHooksGetExecutionAsyncId(v8::Isolate* isolate);
/* legacy alias */
NODE_EXTERN NODE_DEPRECATED("Use AsyncHooksGetExecutionAsyncId(isolate)",
async_uid AsyncHooksGetCurrentId(v8::Isolate* isolate));
async_id AsyncHooksGetCurrentId(v8::Isolate* isolate));


/* Return same value as async_hooks.triggerAsyncId(); */
NODE_EXTERN async_uid AsyncHooksGetTriggerAsyncId(v8::Isolate* isolate);
NODE_EXTERN async_id AsyncHooksGetTriggerAsyncId(v8::Isolate* isolate);
/* legacy alias */
NODE_EXTERN NODE_DEPRECATED("Use AsyncHooksGetTriggerAsyncId(isolate)",
async_uid AsyncHooksGetTriggerId(v8::Isolate* isolate));
async_id AsyncHooksGetTriggerId(v8::Isolate* isolate));


/* If the native API doesn't inherit from the helper class then the callbacks
Expand All @@ -548,13 +552,14 @@ NODE_EXTERN NODE_DEPRECATED("Use AsyncHooksGetTriggerAsyncId(isolate)",
* The `trigger_async_id` parameter should correspond to the resource which is
* creating the new resource, which will usually be the return value of
* `AsyncHooksGetTriggerAsyncId()`. */
NODE_EXTERN async_uid EmitAsyncInit(v8::Isolate* isolate,
v8::Local<v8::Object> resource,
const char* name,
async_uid trigger_async_id);
NODE_EXTERN async_context EmitAsyncInit(v8::Isolate* isolate,
v8::Local<v8::Object> resource,
const char* name,
async_id trigger_async_id = -1);

/* Emit the destroy() callback. */
NODE_EXTERN void EmitAsyncDestroy(v8::Isolate* isolate, async_uid id);
NODE_EXTERN void EmitAsyncDestroy(v8::Isolate* isolate,
async_context asyncContext);

/* An API specific to emit before/after callbacks is unnecessary because
* MakeCallback will automatically call them for you.
Expand All @@ -572,24 +577,21 @@ v8::MaybeLocal<v8::Value> MakeCallback(v8::Isolate* isolate,
v8::Local<v8::Function> callback,
int argc,
v8::Local<v8::Value>* argv,
async_uid asyncId,
async_uid triggerAsyncId);
async_context asyncContext);
NODE_EXTERN
v8::MaybeLocal<v8::Value> MakeCallback(v8::Isolate* isolate,
v8::Local<v8::Object> recv,
const char* method,
int argc,
v8::Local<v8::Value>* argv,
async_uid asyncId,
async_uid triggerAsyncId);
async_context asyncContext);
NODE_EXTERN
v8::MaybeLocal<v8::Value> MakeCallback(v8::Isolate* isolate,
v8::Local<v8::Object> recv,
v8::Local<v8::String> symbol,
int argc,
v8::Local<v8::Value>* argv,
async_uid asyncId,
async_uid triggerAsyncId);
async_context asyncContext);

/* Helper class users can optionally inherit from. If
* `AsyncResource::MakeCallback()` is used, then all four callbacks will be
Expand All @@ -599,18 +601,15 @@ class AsyncResource {
AsyncResource(v8::Isolate* isolate,
v8::Local<v8::Object> resource,
const char* name,
async_uid trigger_async_id = -1)
async_id trigger_async_id = -1)
: isolate_(isolate),
resource_(isolate, resource),
trigger_async_id_(trigger_async_id) {
if (trigger_async_id_ == -1)
trigger_async_id_ = AsyncHooksGetTriggerAsyncId(isolate);

uid_ = EmitAsyncInit(isolate, resource, name, trigger_async_id_);
resource_(isolate, resource) {
async_context_ = EmitAsyncInit(isolate, resource, name,
trigger_async_id);
}

~AsyncResource() {
EmitAsyncDestroy(isolate_, uid_);
EmitAsyncDestroy(isolate_, async_context_);
}

v8::MaybeLocal<v8::Value> MakeCallback(
Expand All @@ -619,7 +618,7 @@ class AsyncResource {
v8::Local<v8::Value>* argv) {
return node::MakeCallback(isolate_, get_resource(),
callback, argc, argv,
uid_, trigger_async_id_);
async_context_);
}

v8::MaybeLocal<v8::Value> MakeCallback(
Expand All @@ -628,7 +627,7 @@ class AsyncResource {
v8::Local<v8::Value>* argv) {
return node::MakeCallback(isolate_, get_resource(),
method, argc, argv,
uid_, trigger_async_id_);
async_context_);
}

v8::MaybeLocal<v8::Value> MakeCallback(
Expand All @@ -637,21 +636,30 @@ class AsyncResource {
v8::Local<v8::Value>* argv) {
return node::MakeCallback(isolate_, get_resource(),
symbol, argc, argv,
uid_, trigger_async_id_);
async_context_);
}

v8::Local<v8::Object> get_resource() {
return resource_.Get(isolate_);
}

async_uid get_uid() const {
return uid_;
NODE_DEPRECATED("Use AsyncResource::get_async_id()",
async_id get_uid() const {
return get_async_id();
}
)

async_id get_async_id() const {
return async_context_.async_id;
}

async_id get_trigger_async_id() const {
return async_context_.trigger_async_id;
}
private:
v8::Isolate* isolate_;
v8::Persistent<v8::Object> resource_;
async_uid uid_;
async_uid trigger_async_id_;
async_context async_context_;
};

} // namespace node
Expand Down
Loading