From 9da102f435158bc12659856523383ad7ca1f44b4 Mon Sep 17 00:00:00 2001 From: Ali Ijaz Sheikh Date: Wed, 7 Feb 2018 17:46:49 -0800 Subject: [PATCH] async_hooks: rename PromiseWrap.parentId Rename the `parentId` property on the PromiseWrap object to a `isChainedPromise` property. The former wasn't quite useful as it was always defined to be the same value as the trigger id available in the init hook. Instead rename the property to be closer to the information it communicates: whether the promise is a chained promise or not. PR-URL: https://github.com/nodejs/node/pull/18633 Fixes: https://github.com/nodejs/node/issues/18470 Reviewed-By: Andreas Madsen Reviewed-By: James M Snell Reviewed-By: Tiancheng "Timothy" Gu --- doc/api/async_hooks.md | 6 +++--- src/async_wrap.cc | 25 +++++++++++------------ test/parallel/test-async-hooks-promise.js | 4 ++-- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/doc/api/async_hooks.md b/doc/api/async_hooks.md index e0922fcd1134f5..2156477ae13c61 100644 --- a/doc/api/async_hooks.md +++ b/doc/api/async_hooks.md @@ -301,10 +301,10 @@ and document their own resource objects. For example, such a resource object could contain the SQL query being executed. 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 -set to the `asyncId` of a parent Promise, if there is one, and `undefined` +that refers to the Promise that is being initialized, and a `isChainedPromise` +property, set to `true` if the promise has a parent promise, and `false` otherwise. For example, in the case of `b = a.then(handler)`, `a` is considered -a parent Promise of `b`. +a parent Promise of `b`. Here, `b` is considered a chained promise. *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. diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 20d21e31df8d5b..c9dfc40c0ca52d 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -239,7 +239,7 @@ class PromiseWrap : public AsyncWrap { size_t self_size() const override { return sizeof(*this); } static constexpr int kPromiseField = 1; - static constexpr int kParentAsyncIdField = 2; + static constexpr int kIsChainedPromiseField = 2; static constexpr int kInternalFieldCount = 3; static PromiseWrap* New(Environment* env, @@ -248,8 +248,8 @@ class PromiseWrap : public AsyncWrap { bool silent); static void GetPromise(Local property, const PropertyCallbackInfo& info); - static void getParentAsyncId(Local property, - const PropertyCallbackInfo& info); + static void getIsChainedPromise(Local property, + const PropertyCallbackInfo& info); }; PromiseWrap* PromiseWrap::New(Environment* env, @@ -259,11 +259,10 @@ PromiseWrap* PromiseWrap::New(Environment* env, Local object = env->promise_wrap_template() ->NewInstance(env->context()).ToLocalChecked(); object->SetInternalField(PromiseWrap::kPromiseField, promise); - if (parent_wrap != nullptr) { - object->SetInternalField(PromiseWrap::kParentAsyncIdField, - Number::New(env->isolate(), - parent_wrap->get_async_id())); - } + object->SetInternalField(PromiseWrap::kIsChainedPromiseField, + parent_wrap != nullptr ? + v8::True(env->isolate()) : + v8::False(env->isolate())); CHECK_EQ(promise->GetAlignedPointerFromInternalField(0), nullptr); promise->SetInternalField(0, object); return new PromiseWrap(env, object, silent); @@ -274,10 +273,10 @@ void PromiseWrap::GetPromise(Local property, info.GetReturnValue().Set(info.Holder()->GetInternalField(kPromiseField)); } -void PromiseWrap::getParentAsyncId(Local property, - const PropertyCallbackInfo& info) { +void PromiseWrap::getIsChainedPromise(Local property, + const PropertyCallbackInfo& info) { info.GetReturnValue().Set( - info.Holder()->GetInternalField(kParentAsyncIdField)); + info.Holder()->GetInternalField(kIsChainedPromiseField)); } static void PromiseHook(PromiseHookType type, Local promise, @@ -377,8 +376,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(), "parentId"), - PromiseWrap::getParentAsyncId); + FIXED_ONE_BYTE_STRING(env->isolate(), "isChainedPromise"), + PromiseWrap::getIsChainedPromise); 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 d712fd616c647b..4b36f6026b36c6 100644 --- a/test/parallel/test-async-hooks-promise.js +++ b/test/parallel/test-async-hooks-promise.js @@ -21,8 +21,8 @@ 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.isChainedPromise, false); 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.isChainedPromise, true); assert.strictEqual(initCalls[1].resource.promise, b);