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.

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

async_wrap,src: promise hook integration #13000

Closed
wants to merge 16 commits into from
17 changes: 11 additions & 6 deletions lib/async_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,7 @@ const before_symbol = Symbol('before');
const after_symbol = Symbol('after');
const destroy_symbol = Symbol('destroy');

// Setup the callbacks that node::AsyncWrap will call when there are hooks to
// process. They use the same functions as the JS embedder API.
async_wrap.setupHooks({ init,
before: emitBeforeN,
after: emitAfterN,
destroy: emitDestroyN });
let setupHooksCalled = false;

// Used to fatally abort the process if a callback throws.
function fatalError(e) {
Expand Down Expand Up @@ -103,6 +98,16 @@ class AsyncHook {
if (hooks_array.includes(this))
return;

if (!setupHooksCalled) {
setupHooksCalled = true;
// Setup the callbacks that node::AsyncWrap will call when there are
// hooks to process. They use the same functions as the JS embedder API.
async_wrap.setupHooks({ init,
before: emitBeforeN,
after: emitAfterN,
destroy: emitDestroyN });
}

// createHook() has already enforced that the callbacks are all functions,
// so here simply increment the count of whether each callbacks exists or
// not.
Expand Down
218 changes: 153 additions & 65 deletions src/async-wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ using v8::Local;
using v8::MaybeLocal;
using v8::Number;
using v8::Object;
using v8::Promise;
using v8::PromiseHookType;
using v8::RetainedObjectInfo;
using v8::Symbol;
using v8::TryCatch;
Expand Down Expand Up @@ -177,6 +179,143 @@ static void PushBackDestroyId(Environment* env, double id) {
}


bool DomainEnter(Environment* env, Local<Object> object) {
Local<Value> domain_v = object->Get(env->domain_string());
if (domain_v->IsObject()) {
Local<Object> domain = domain_v.As<Object>();
if (domain->Get(env->disposed_string())->IsTrue())
return true;
Local<Value> enter_v = domain->Get(env->enter_string());
if (enter_v->IsFunction()) {
if (enter_v.As<Function>()->Call(domain, 0, nullptr).IsEmpty()) {
FatalError("node::AsyncWrap::MakeCallback",
"domain enter callback threw, please report this");
}
}
}
return false;
}


bool DomainExit(Environment* env, v8::Local<v8::Object> object) {
Local<Value> domain_v = object->Get(env->domain_string());
if (domain_v->IsObject()) {
Local<Object> domain = domain_v.As<Object>();
if (domain->Get(env->disposed_string())->IsTrue())
return true;
Local<Value> exit_v = domain->Get(env->exit_string());
if (exit_v->IsFunction()) {
if (exit_v.As<Function>()->Call(domain, 0, nullptr).IsEmpty()) {
FatalError("node::AsyncWrap::MakeCallback",
"domain exit callback threw, please report this");
}
}
}
return false;
}


static bool PreCallbackExecution(AsyncWrap* wrap, bool run_domain_cbs) {
AsyncHooks* async_hooks = wrap->env()->async_hooks();

if (wrap->env()->using_domains() && run_domain_cbs) {
bool is_disposed = DomainEnter(wrap->env(), wrap->object());
if (is_disposed)
return false;
}

if (async_hooks->fields()[AsyncHooks::kBefore] > 0) {
Local<Value> uid = Number::New(wrap->env()->isolate(), wrap->get_id());
Local<Function> fn = wrap->env()->async_hooks_before_function();
TryCatch try_catch(wrap->env()->isolate());
MaybeLocal<Value> ar = fn->Call(
wrap->env()->context(), Undefined(wrap->env()->isolate()), 1, &uid);
if (ar.IsEmpty()) {
ClearFatalExceptionHandlers(wrap->env());
FatalException(wrap->env()->isolate(), try_catch);
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

FatalException() always exits the application in this case because ClearFatalException() removed all unhandledException callbacks. So I'll assume this return is so the compiler doesn't emit a warning.

I decided this would be the safest course of action when first implementing AsyncWrap because it was difficult to properly clean up if an async hook threw. It was difficult enough to properly cleanup if a user's callback threw (see process._fatalException() in lib/internal/bootstrap_node.js). If we can properly show it's safe to recover from an async hook throwing then I'm all for changing this behavior.

Copy link
Author

Choose a reason for hiding this comment

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

That makes sense, maybe we can look into that more in a follow on pr.

}
}

return true;
}


static bool PostCallbackExecution(AsyncWrap* wrap, bool run_domain_cbs) {
AsyncHooks* async_hooks = wrap->env()->async_hooks();

// If the callback failed then the after() hooks will be called at the end
// of _fatalException().
if (async_hooks->fields()[AsyncHooks::kAfter] > 0) {
Local<Value> uid = Number::New(wrap->env()->isolate(), wrap->get_id());
Local<Function> fn = wrap->env()->async_hooks_after_function();
TryCatch try_catch(wrap->env()->isolate());
MaybeLocal<Value> ar = fn->Call(
wrap->env()->context(), Undefined(wrap->env()->isolate()), 1, &uid);
if (ar.IsEmpty()) {
ClearFatalExceptionHandlers(wrap->env());
FatalException(wrap->env()->isolate(), try_catch);
return false;
}
}

if (wrap->env()->using_domains() && run_domain_cbs) {
bool is_disposed = DomainExit(wrap->env(), wrap->object());
if (is_disposed)
return false;
}

return true;
}

class PromiseWrap : public AsyncWrap {
public:
PromiseWrap(Environment* env, Local<Object> object)
: AsyncWrap(env, object, PROVIDER_PROMISE) {}
size_t self_size() const override { return sizeof(*this); }
};


static void PromiseHook(PromiseHookType type, Local<Promise> promise,
Local<Value> parent, void* arg) {
Local<Context> context = promise->CreationContext();
Environment* env = Environment::GetCurrent(context);
if (type == PromiseHookType::kInit) {
// Unfortunately, promises don't have internal fields. Need a surrogate that
// async wrap can wrap.
Local<Object> obj =
env->async_hooks_promise_object()->NewInstance(context).ToLocalChecked();
PromiseWrap* wrap = new PromiseWrap(env, obj);
v8::PropertyAttribute hidden =
static_cast<v8::PropertyAttribute>(v8::ReadOnly
| v8::DontDelete
| v8::DontEnum);
promise->DefineOwnProperty(context,
env->promise_wrap(),
v8::External::New(env->isolate(), wrap),
hidden).FromJust();
// The async tag will be destroyed at the same time as the promise as the
// only reference to it is held by the promise. This allows the promise
// wrap instance to be notified when the promise is destroyed.
promise->DefineOwnProperty(context,
env->promise_async_tag(),
obj, hidden).FromJust();
} else if (type == PromiseHookType::kResolve) {
// TODO(matthewloring): need to expose this through the async hooks api.
}
Local<v8::Value> external_wrap =
promise->Get(context, env->promise_wrap()).ToLocalChecked();
PromiseWrap* wrap =
static_cast<PromiseWrap*>(external_wrap.As<v8::External>()->Value());
CHECK_NE(wrap, nullptr);
if (type == PromiseHookType::kBefore) {
PreCallbackExecution(wrap, false);
} else if (type == PromiseHookType::kAfter) {
PostCallbackExecution(wrap, false);
}
}


static void SetupHooks(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

Expand All @@ -201,6 +340,7 @@ static void SetupHooks(const FunctionCallbackInfo<Value>& args) {
SET_HOOK_FN(before);
SET_HOOK_FN(after);
SET_HOOK_FN(destroy);
env->AddPromiseHook(PromiseHook, nullptr);
#undef SET_HOOK_FN
}

Expand Down Expand Up @@ -262,6 +402,11 @@ void AsyncWrap::Initialize(Local<Object> target,
env->SetMethod(target, "clearIdStack", ClearIdStack);
env->SetMethod(target, "addIdToDestroyList", QueueDestroyId);

Local<v8::ObjectTemplate> promise_object_template =
v8::ObjectTemplate::New(env->isolate());
promise_object_template->SetInternalFieldCount(1);
env->set_async_hooks_promise_object(promise_object_template);

v8::PropertyAttribute ReadOnlyDontDelete =
static_cast<v8::PropertyAttribute>(v8::ReadOnly | v8::DontDelete);

Expand Down Expand Up @@ -416,87 +561,30 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
Local<Value>* argv) {
CHECK(env()->context() == env()->isolate()->GetCurrentContext());

AsyncHooks* async_hooks = env()->async_hooks();
Local<Object> context = object();
Local<Object> domain;
Local<Value> uid;
bool has_domain = false;

Environment::AsyncCallbackScope callback_scope(env());

if (env()->using_domains()) {
Local<Value> domain_v = context->Get(env()->domain_string());
has_domain = domain_v->IsObject();
if (has_domain) {
domain = domain_v.As<Object>();
if (domain->Get(env()->disposed_string())->IsTrue())
return Local<Value>();
}
}
Environment::AsyncHooks::ExecScope exec_scope(env(),
get_id(),
get_trigger_id());

if (has_domain) {
Local<Value> enter_v = domain->Get(env()->enter_string());
if (enter_v->IsFunction()) {
if (enter_v.As<Function>()->Call(domain, 0, nullptr).IsEmpty()) {
FatalError("node::AsyncWrap::MakeCallback",
"domain enter callback threw, please report this");
}
}
}

// Want currentId() to return the correct value from the callbacks.
AsyncHooks::ExecScope exec_scope(env(), get_id(), get_trigger_id());

if (async_hooks->fields()[AsyncHooks::kBefore] > 0) {
uid = Number::New(env()->isolate(), get_id());
Local<Function> fn = env()->async_hooks_before_function();
TryCatch try_catch(env()->isolate());
MaybeLocal<Value> ar = fn->Call(
env()->context(), Undefined(env()->isolate()), 1, &uid);
if (ar.IsEmpty()) {
ClearFatalExceptionHandlers(env());
FatalException(env()->isolate(), try_catch);
return Local<Value>();
}
if (!PreCallbackExecution(this, true)) {
return Local<Value>();
}

// Finally... Get to running the user's callback.
MaybeLocal<Value> ret = cb->Call(env()->context(), context, argc, argv);
MaybeLocal<Value> ret = cb->Call(env()->context(), object(), argc, argv);

Local<Value> ret_v;
if (!ret.ToLocal(&ret_v)) {
return Local<Value>();
}

// If the callback failed then the after() hooks will be called at the end
// of _fatalException().
if (async_hooks->fields()[AsyncHooks::kAfter] > 0) {
if (uid.IsEmpty())
uid = Number::New(env()->isolate(), get_id());
Local<Function> fn = env()->async_hooks_after_function();
TryCatch try_catch(env()->isolate());
MaybeLocal<Value> ar = fn->Call(
env()->context(), Undefined(env()->isolate()), 1, &uid);
if (ar.IsEmpty()) {
ClearFatalExceptionHandlers(env());
FatalException(env()->isolate(), try_catch);
return Local<Value>();
}
if (!PostCallbackExecution(this, true)) {
return Local<Value>();
}

// The execution scope of the id and trigger_id only go this far.
exec_scope.Dispose();

if (has_domain) {
Local<Value> exit_v = domain->Get(env()->exit_string());
if (exit_v->IsFunction()) {
if (exit_v.As<Function>()->Call(domain, 0, nullptr).IsEmpty()) {
FatalError("node::AsyncWrap::MakeCallback",
"domain exit callback threw, please report this");
}
}
}

if (callback_scope.in_makecallback()) {
return ret_v;
}
Expand Down
4 changes: 4 additions & 0 deletions src/async-wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ namespace node {
V(PIPECONNECTWRAP) \
V(PIPEWRAP) \
V(PROCESSWRAP) \
V(PROMISE) \
V(QUERYWRAP) \
V(RANDOMBYTESREQUEST) \
V(SHUTDOWNWRAP) \
Expand Down Expand Up @@ -123,6 +124,9 @@ class AsyncWrap : public BaseObject {

void LoadAsyncWrapperInfo(Environment* env);

bool DomainEnter(Environment* env, v8::Local<v8::Object> object);
bool DomainExit(Environment* env, v8::Local<v8::Object> object);

} // namespace node

#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
Expand Down
3 changes: 3 additions & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,8 @@ namespace node {
V(preference_string, "preference") \
V(priority_string, "priority") \
V(produce_cached_data_string, "produceCachedData") \
V(promise_wrap, "_promise_async_wrap") \
V(promise_async_tag, "_promise_async_wrap_tag") \
V(raw_string, "raw") \
V(read_host_object_string, "_readHostObject") \
V(readable_string, "readable") \
Expand Down Expand Up @@ -256,6 +258,7 @@ namespace node {
V(async_hooks_init_function, v8::Function) \
V(async_hooks_before_function, v8::Function) \
V(async_hooks_after_function, v8::Function) \
V(async_hooks_promise_object, v8::ObjectTemplate) \
V(binding_cache_object, v8::Object) \
V(buffer_constructor_function, v8::Function) \
V(buffer_prototype_object, v8::Object) \
Expand Down
35 changes: 3 additions & 32 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1124,7 +1124,6 @@ void DomainPromiseHook(PromiseHookType type,
Environment* env = static_cast<Environment*>(arg);
Local<Context> context = env->context();

if (type == PromiseHookType::kResolve) return;
if (type == PromiseHookType::kInit && env->in_domain()) {
promise->Set(context,
env->domain_string(),
Expand All @@ -1133,38 +1132,10 @@ void DomainPromiseHook(PromiseHookType type,
return;
}

// Loosely based on node::MakeCallback().
Local<Value> domain_v =
promise->Get(context, env->domain_string()).ToLocalChecked();
if (!domain_v->IsObject())
return;

Local<Object> domain = domain_v.As<Object>();
if (domain->Get(context, env->disposed_string())
.ToLocalChecked()->IsTrue()) {
return;
}

if (type == PromiseHookType::kBefore) {
Local<Value> enter_v =
domain->Get(context, env->enter_string()).ToLocalChecked();
if (enter_v->IsFunction()) {
if (enter_v.As<Function>()->Call(context, domain, 0, nullptr).IsEmpty()) {
FatalError("node::PromiseHook",
"domain enter callback threw, please report this "
"as a bug in Node.js");
}
}
} else {
Local<Value> exit_v =
domain->Get(context, env->exit_string()).ToLocalChecked();
if (exit_v->IsFunction()) {
if (exit_v.As<Function>()->Call(context, domain, 0, nullptr).IsEmpty()) {
FatalError("node::MakeCallback",
"domain exit callback threw, please report this "
"as a bug in Node.js");
}
}
DomainEnter(env, promise);
} else if (type == PromiseHookType::kAfter) {
DomainExit(env, promise);
}
}

Expand Down
Loading