Skip to content

Commit

Permalink
async_hooks: rename PromiseWrap.parentId
Browse files Browse the repository at this point in the history
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: nodejs#18633
Fixes: nodejs#18470
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
  • Loading branch information
ofrobots committed Feb 12, 2018
1 parent bd4773a commit 9f6a565
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 18 deletions.
6 changes: 3 additions & 3 deletions doc/api/async_hooks.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

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.
Expand Down
25 changes: 12 additions & 13 deletions src/async_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,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,
Expand All @@ -254,8 +254,8 @@ class PromiseWrap : public AsyncWrap {
bool silent);
static void GetPromise(Local<String> property,
const PropertyCallbackInfo<Value>& info);
static void getParentAsyncId(Local<String> property,
const PropertyCallbackInfo<Value>& info);
static void getIsChainedPromise(Local<String> property,
const PropertyCallbackInfo<Value>& info);
};

PromiseWrap* PromiseWrap::New(Environment* env,
Expand All @@ -265,11 +265,10 @@ PromiseWrap* PromiseWrap::New(Environment* env,
Local<Object> 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);
Expand All @@ -280,10 +279,10 @@ void PromiseWrap::GetPromise(Local<String> property,
info.GetReturnValue().Set(info.Holder()->GetInternalField(kPromiseField));
}

void PromiseWrap::getParentAsyncId(Local<String> property,
const PropertyCallbackInfo<Value>& info) {
void PromiseWrap::getIsChainedPromise(Local<String> property,
const PropertyCallbackInfo<Value>& info) {
info.GetReturnValue().Set(
info.Holder()->GetInternalField(kParentAsyncIdField));
info.Holder()->GetInternalField(kIsChainedPromiseField));
}

static void PromiseHook(PromiseHookType type, Local<Promise> promise,
Expand Down Expand Up @@ -383,8 +382,8 @@ static void SetupHooks(const FunctionCallbackInfo<Value>& 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);
}
}
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-async-hooks-promise.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

0 comments on commit 9f6a565

Please sign in to comment.