From a187daad7fc19f1526e865416da8b30ff206fb8d Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 4 Jun 2017 13:35:56 +0200 Subject: [PATCH 1/5] 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. --- src/async-wrap.cc | 90 +++++++++++++++++++++-- src/env.h | 1 + test/parallel/test-async-hooks-promise.js | 25 +++++++ 3 files changed, 108 insertions(+), 8 deletions(-) create mode 100644 test/parallel/test-async-hooks-promise.js diff --git a/src/async-wrap.cc b/src/async-wrap.cc index 0f5c800f40d939..b25a47c1e28d8a 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,91 @@ 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 kParentField = 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 GetParent(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); + object->SetAlignedPointerInInternalField( + PromiseWrap::kParentField, + static_cast(parent_wrap)); + 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::GetParent(Local property, + const PropertyCallbackInfo& info) { + Isolate* isolate = info.GetIsolate(); + PromiseWrap* parent = static_cast( + info.Holder()->GetAlignedPointerFromInternalField(kParentField)); + if (parent == nullptr) + info.GetReturnValue().Set(Undefined(isolate)); + else + info.GetReturnValue().Set(parent->object()); +} 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); + Local resource_object; + PromiseWrap* wrap = nullptr; + if (resource_object_value->IsObject()) { + 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 +408,23 @@ 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(), "parent"), + PromiseWrap::GetParent); + env->SetProtoMethod(ctor, "getAsyncId", AsyncWrap::GetAsyncId); + env->set_promise_wrap_template(promise_wrap_template); + } } diff --git a/src/env.h b/src/env.h index 7179a6081cc417..2eacf68ef5e93b 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 00000000000000..d9cd13a99ac2b0 --- /dev/null +++ b/test/parallel/test-async-hooks-promise.js @@ -0,0 +1,25 @@ +'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.parent, undefined); +assert.strictEqual(initCalls[0].resource.promise, a); +assert.strictEqual(initCalls[0].resource.getAsyncId(), initCalls[0].id); +assert.strictEqual(initCalls[1].triggerId, initCalls[0].id); +assert.strictEqual(initCalls[1].resource.parent, initCalls[0].resource); +assert.strictEqual(initCalls[1].resource.promise, b); +assert.strictEqual(initCalls[1].resource.getAsyncId(), initCalls[1].id); From 0875682a3062e82f27b5a50116de0212dd939d69 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 4 Jun 2017 17:00:55 +0200 Subject: [PATCH 2/5] [squash] provide parent id rather than parent resource --- src/async-wrap.cc | 30 ++++++++++------------- test/parallel/test-async-hooks-promise.js | 4 +-- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/src/async-wrap.cc b/src/async-wrap.cc index b25a47c1e28d8a..4f9ecbbeb437af 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -291,7 +291,7 @@ class PromiseWrap : public AsyncWrap { size_t self_size() const override { return sizeof(*this); } static constexpr int kPromiseField = 1; - static constexpr int kParentField = 2; + static constexpr int kParentIdField = 2; static constexpr int kInternalFieldCount = 3; static PromiseWrap* New(Environment* env, @@ -300,8 +300,8 @@ class PromiseWrap : public AsyncWrap { bool silent); static void GetPromise(Local property, const PropertyCallbackInfo& info); - static void GetParent(Local property, - const PropertyCallbackInfo& info); + static void GetParentId(Local property, + const PropertyCallbackInfo& info); }; PromiseWrap* PromiseWrap::New(Environment* env, @@ -311,9 +311,11 @@ PromiseWrap* PromiseWrap::New(Environment* env, Local object = env->promise_wrap_template() ->NewInstance(env->context()).ToLocalChecked(); object->SetInternalField(PromiseWrap::kPromiseField, promise); - object->SetAlignedPointerInInternalField( - PromiseWrap::kParentField, - static_cast(parent_wrap)); + 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); @@ -324,15 +326,9 @@ void PromiseWrap::GetPromise(Local property, info.GetReturnValue().Set(info.Holder()->GetInternalField(kPromiseField)); } -void PromiseWrap::GetParent(Local property, - const PropertyCallbackInfo& info) { - Isolate* isolate = info.GetIsolate(); - PromiseWrap* parent = static_cast( - info.Holder()->GetAlignedPointerFromInternalField(kParentField)); - if (parent == nullptr) - info.GetReturnValue().Set(Undefined(isolate)); - else - info.GetReturnValue().Set(parent->object()); +void PromiseWrap::GetParentId(Local property, + const PropertyCallbackInfo& info) { + info.GetReturnValue().Set(info.Holder()->GetInternalField(kParentIdField)); } static void PromiseHook(PromiseHookType type, Local promise, @@ -420,8 +416,8 @@ static void SetupHooks(const FunctionCallbackInfo& args) { FIXED_ONE_BYTE_STRING(env->isolate(), "promise"), PromiseWrap::GetPromise); promise_wrap_template->SetAccessor( - FIXED_ONE_BYTE_STRING(env->isolate(), "parent"), - PromiseWrap::GetParent); + FIXED_ONE_BYTE_STRING(env->isolate(), "parentId"), + PromiseWrap::GetParentId); env->SetProtoMethod(ctor, "getAsyncId", AsyncWrap::GetAsyncId); env->set_promise_wrap_template(promise_wrap_template); } diff --git a/test/parallel/test-async-hooks-promise.js b/test/parallel/test-async-hooks-promise.js index d9cd13a99ac2b0..4953a5bd81a5d1 100644 --- a/test/parallel/test-async-hooks-promise.js +++ b/test/parallel/test-async-hooks-promise.js @@ -16,10 +16,10 @@ const a = Promise.resolve(42); const b = a.then(common.mustCall()); assert.strictEqual(initCalls[0].triggerId, 1); -assert.strictEqual(initCalls[0].resource.parent, undefined); +assert.strictEqual(initCalls[0].resource.parentId, undefined); assert.strictEqual(initCalls[0].resource.promise, a); assert.strictEqual(initCalls[0].resource.getAsyncId(), initCalls[0].id); assert.strictEqual(initCalls[1].triggerId, initCalls[0].id); -assert.strictEqual(initCalls[1].resource.parent, initCalls[0].resource); +assert.strictEqual(initCalls[1].resource.parentId, initCalls[0].id); assert.strictEqual(initCalls[1].resource.promise, b); assert.strictEqual(initCalls[1].resource.getAsyncId(), initCalls[1].id); From 464d2af369193020ab995668aa9f01692348c0c9 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 4 Jun 2017 17:12:40 +0200 Subject: [PATCH 3/5] [squash] remove getAsyncId --- src/async-wrap.cc | 1 - test/parallel/test-async-hooks-promise.js | 2 -- 2 files changed, 3 deletions(-) diff --git a/src/async-wrap.cc b/src/async-wrap.cc index 4f9ecbbeb437af..739442de17c7f1 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -418,7 +418,6 @@ static void SetupHooks(const FunctionCallbackInfo& args) { promise_wrap_template->SetAccessor( FIXED_ONE_BYTE_STRING(env->isolate(), "parentId"), PromiseWrap::GetParentId); - env->SetProtoMethod(ctor, "getAsyncId", AsyncWrap::GetAsyncId); env->set_promise_wrap_template(promise_wrap_template); } } diff --git a/test/parallel/test-async-hooks-promise.js b/test/parallel/test-async-hooks-promise.js index 4953a5bd81a5d1..ea5e59d319db53 100644 --- a/test/parallel/test-async-hooks-promise.js +++ b/test/parallel/test-async-hooks-promise.js @@ -18,8 +18,6 @@ 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[0].resource.getAsyncId(), initCalls[0].id); assert.strictEqual(initCalls[1].triggerId, initCalls[0].id); assert.strictEqual(initCalls[1].resource.parentId, initCalls[0].id); assert.strictEqual(initCalls[1].resource.promise, b); -assert.strictEqual(initCalls[1].resource.getAsyncId(), initCalls[1].id); From 2652910a6d1ffe0fb0f6c33bdaca6b5efdfb7522 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 4 Jun 2017 17:34:08 +0200 Subject: [PATCH 4/5] [squash] add docs --- doc/api/async_hooks.md | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/doc/api/async_hooks.md b/doc/api/async_hooks.md index 77a6c165989229..84c9dd45b34975 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 From 4acaed3c661214fdf86b0d4fa477bb0ff9e2eb32 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 8 Jun 2017 20:36:04 +0200 Subject: [PATCH 5/5] [squash] address nit --- src/async-wrap.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/async-wrap.cc b/src/async-wrap.cc index 739442de17c7f1..d335052a8fc9fd 100644 --- a/src/async-wrap.cc +++ b/src/async-wrap.cc @@ -336,10 +336,9 @@ static void PromiseHook(PromiseHookType type, Local promise, Local context = promise->CreationContext(); Environment* env = Environment::GetCurrent(context); Local resource_object_value = promise->GetInternalField(0); - Local resource_object; PromiseWrap* wrap = nullptr; if (resource_object_value->IsObject()) { - resource_object = resource_object_value.As(); + Local resource_object = resource_object_value.As(); wrap = Unwrap(resource_object); } if (type == PromiseHookType::kInit || wrap == nullptr) {