From 8f39881b743d65a0be8bd4ab0b4574f7109bcd07 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 4 Jun 2017 13:35:56 +0200 Subject: [PATCH] async_hooks: use resource objects for Promises MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use `PromiseWrap` resource objects whose lifetimes are tied to the `Promise` instances themselves to track promises, and have a `.promise` getter that points to the `Promise` and a `.parent` property that points to the parent Promise’s resource object, if there is any. The properties are implemented as getters for internal fields rather than normal properties in the hope that it helps keep performance for the common case that async_hooks users will often not inspect them. PR-URL: https://github.com/nodejs/node/pull/13452 Reviewed-By: Andreas Madsen Reviewed-By: Trevor Norris --- doc/api/async_hooks.md | 13 +++- src/async-wrap.cc | 84 ++++++++++++++++++++--- src/env.h | 1 + test/parallel/test-async-hooks-promise.js | 23 +++++++ 4 files changed, 111 insertions(+), 10 deletions(-) create mode 100644 test/parallel/test-async-hooks-promise.js diff --git a/doc/api/async_hooks.md b/doc/api/async_hooks.md index 77a6c165989..84c9dd45b34 100644 --- a/doc/api/async_hooks.md +++ b/doc/api/async_hooks.md @@ -203,13 +203,16 @@ UDPSENDWRAP, UDPWRAP, WRITEWRAP, ZLIB, SSLCONNECTION, PBKDF2REQUEST, RANDOMBYTESREQUEST, TLSWRAP ``` +There is also the `PROMISE` resource type, which is used to track `Promise` +instances and asynchronous work scheduled by them. + Users are be able to define their own `type` when using the public embedder API. *Note:* It is possible to have type name collisions. Embedders are encouraged to use a unique prefixes, such as the npm package name, to prevent collisions when listening to the hooks. -###### `triggerid` +###### `triggerId` `triggerId` is the `asyncId` of the resource that caused (or "triggered") the new resource to initialize and that caused `init` to call. This is different @@ -258,7 +261,13 @@ considered public, but using the Embedder API users can provide and document their own resource objects. Such as resource object could for example contain the SQL query being executed. -*Note:* In some cases the resource object is reused for performance reasons, +In the case of Promises, the `resource` object will have `promise` property +that refers to the Promise that is being initialized, and a `parentId` property +that equals the `asyncId` of a parent Promise, if there is one, and +`undefined` otherwise. For example, in the case of `b = a.then(handler)`, +`a` is considered a parent Promise of `b`. + +*Note*: In some cases the resource object is reused for performance reasons, it is thus not safe to use it as a key in a `WeakMap` or add properties to it. ###### asynchronous context example diff --git a/src/async-wrap.cc b/src/async-wrap.cc index 813d7c21bb0..2dcfdc79972 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -36,6 +36,7 @@ using v8::Context; using v8::Float64Array; using v8::Function; using v8::FunctionCallbackInfo; +using v8::FunctionTemplate; using v8::HandleScope; using v8::HeapProfiler; using v8::Integer; @@ -44,8 +45,10 @@ using v8::Local; using v8::MaybeLocal; using v8::Number; using v8::Object; +using v8::ObjectTemplate; using v8::Promise; using v8::PromiseHookType; +using v8::PropertyCallbackInfo; using v8::RetainedObjectInfo; using v8::String; using v8::Symbol; @@ -282,37 +285,86 @@ bool AsyncWrap::EmitAfter(Environment* env, double async_id) { class PromiseWrap : public AsyncWrap { public: PromiseWrap(Environment* env, Local object, bool silent) - : AsyncWrap(env, object, PROVIDER_PROMISE, silent) {} + : AsyncWrap(env, object, PROVIDER_PROMISE, silent) { + MakeWeak(this); + } size_t self_size() const override { return sizeof(*this); } + + static constexpr int kPromiseField = 1; + static constexpr int kParentIdField = 2; + static constexpr int kInternalFieldCount = 3; + + static PromiseWrap* New(Environment* env, + Local promise, + PromiseWrap* parent_wrap, + bool silent); + static void GetPromise(Local property, + const PropertyCallbackInfo& info); + static void GetParentId(Local property, + const PropertyCallbackInfo& info); }; +PromiseWrap* PromiseWrap::New(Environment* env, + Local promise, + PromiseWrap* parent_wrap, + bool silent) { + Local object = env->promise_wrap_template() + ->NewInstance(env->context()).ToLocalChecked(); + object->SetInternalField(PromiseWrap::kPromiseField, promise); + if (parent_wrap != nullptr) { + object->SetInternalField(PromiseWrap::kParentIdField, + Number::New(env->isolate(), + parent_wrap->get_id())); + } + CHECK_EQ(promise->GetAlignedPointerFromInternalField(0), nullptr); + promise->SetInternalField(0, object); + return new PromiseWrap(env, object, silent); +} + +void PromiseWrap::GetPromise(Local property, + const PropertyCallbackInfo& info) { + info.GetReturnValue().Set(info.Holder()->GetInternalField(kPromiseField)); +} + +void PromiseWrap::GetParentId(Local property, + const PropertyCallbackInfo& info) { + info.GetReturnValue().Set(info.Holder()->GetInternalField(kParentIdField)); +} static void PromiseHook(PromiseHookType type, Local promise, Local parent, void* arg) { Local context = promise->CreationContext(); Environment* env = Environment::GetCurrent(context); - PromiseWrap* wrap = Unwrap(promise); + Local resource_object_value = promise->GetInternalField(0); + PromiseWrap* wrap = nullptr; + if (resource_object_value->IsObject()) { + Local resource_object = resource_object_value.As(); + wrap = Unwrap(resource_object); + } if (type == PromiseHookType::kInit || wrap == nullptr) { bool silent = type != PromiseHookType::kInit; + PromiseWrap* parent_wrap = nullptr; + // set parent promise's async Id as this promise's triggerId if (parent->IsPromise()) { // parent promise exists, current promise // is a chained promise, so we set parent promise's id as // current promise's triggerId Local parent_promise = parent.As(); - auto parent_wrap = Unwrap(parent_promise); + Local parent_resource = parent_promise->GetInternalField(0); + if (parent_resource->IsObject()) { + parent_wrap = Unwrap(parent_resource.As()); + } if (parent_wrap == nullptr) { - // create a new PromiseWrap for parent promise with silent parameter - parent_wrap = new PromiseWrap(env, parent_promise, true); - parent_wrap->MakeWeak(parent_wrap); + parent_wrap = PromiseWrap::New(env, parent_promise, nullptr, true); } // get id from parentWrap double trigger_id = parent_wrap->get_id(); env->set_init_trigger_id(trigger_id); } - wrap = new PromiseWrap(env, promise, silent); - wrap->MakeWeak(wrap); + + wrap = PromiseWrap::New(env, promise, parent_wrap, silent); } else if (type == PromiseHookType::kResolve) { // TODO(matthewloring): need to expose this through the async hooks api. } @@ -351,6 +403,22 @@ static void SetupHooks(const FunctionCallbackInfo& args) { SET_HOOK_FN(destroy); env->AddPromiseHook(PromiseHook, nullptr); #undef SET_HOOK_FN + + { + Local ctor = + FunctionTemplate::New(env->isolate()); + ctor->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "PromiseWrap")); + Local promise_wrap_template = ctor->InstanceTemplate(); + promise_wrap_template->SetInternalFieldCount( + PromiseWrap::kInternalFieldCount); + promise_wrap_template->SetAccessor( + FIXED_ONE_BYTE_STRING(env->isolate(), "promise"), + PromiseWrap::GetPromise); + promise_wrap_template->SetAccessor( + FIXED_ONE_BYTE_STRING(env->isolate(), "parentId"), + PromiseWrap::GetParentId); + env->set_promise_wrap_template(promise_wrap_template); + } } diff --git a/src/env.h b/src/env.h index 7179a6081cc..2eacf68ef5e 100644 --- a/src/env.h +++ b/src/env.h @@ -268,6 +268,7 @@ namespace node { V(pipe_constructor_template, v8::FunctionTemplate) \ V(process_object, v8::Object) \ V(promise_reject_function, v8::Function) \ + V(promise_wrap_template, v8::ObjectTemplate) \ V(push_values_to_array_function, v8::Function) \ V(randombytes_constructor_template, v8::ObjectTemplate) \ V(script_context_constructor_template, v8::FunctionTemplate) \ diff --git a/test/parallel/test-async-hooks-promise.js b/test/parallel/test-async-hooks-promise.js new file mode 100644 index 00000000000..ea5e59d319d --- /dev/null +++ b/test/parallel/test-async-hooks-promise.js @@ -0,0 +1,23 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const async_hooks = require('async_hooks'); + +const initCalls = []; + +async_hooks.createHook({ + init: common.mustCall((id, type, triggerId, resource) => { + assert.strictEqual(type, 'PROMISE'); + initCalls.push({id, triggerId, resource}); + }, 2) +}).enable(); + +const a = Promise.resolve(42); +const b = a.then(common.mustCall()); + +assert.strictEqual(initCalls[0].triggerId, 1); +assert.strictEqual(initCalls[0].resource.parentId, undefined); +assert.strictEqual(initCalls[0].resource.promise, a); +assert.strictEqual(initCalls[1].triggerId, initCalls[0].id); +assert.strictEqual(initCalls[1].resource.parentId, initCalls[0].id); +assert.strictEqual(initCalls[1].resource.promise, b);