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_hooks: use parent promise as triggerId in init hook #13367

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/async-wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,23 @@ static void PromiseHook(PromiseHookType type, Local<Promise> promise,
PromiseWrap* wrap = Unwrap<PromiseWrap>(promise);
if (type == PromiseHookType::kInit || wrap == nullptr) {
bool silent = type != PromiseHookType::kInit;
// 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<Promise> parent_promise = parent.As<Promise>();
auto parent_wrap = Unwrap<PromiseWrap>(parent_promise);

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);
}
// 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);
} else if (type == PromiseHookType::kResolve) {
Expand Down
38 changes: 38 additions & 0 deletions test/async-hooks/test-promise.chain-promise-before-init-hooks.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const initHooks = require('./init-hooks');
const { checkInvocations } = require('./hook-checks');

const p = new Promise(common.mustCall(function executor(resolve, reject) {
resolve(5);
}));

p.then(function afterresolution(val) {
assert.strictEqual(val, 5);
return val;
});

// init hooks after chained promise is created
const hooks = initHooks();
hooks._allowNoInit = true;
hooks.enable();


process.on('exit', function onexit() {
hooks.disable();
hooks.sanityCheck('PROMISE');

// Because the init event was never emitted the hooks listener doesn't
// know what the type was. Thus check for Unknown rather than PROMISE.
const as = hooks.activitiesOfTypes('PROMISE');
const unknown = hooks.activitiesOfTypes('Unknown');
assert.strictEqual(as.length, 0);
assert.strictEqual(unknown.length, 1);

const a0 = unknown[0];
assert.strictEqual(a0.type, 'Unknown');
assert.strictEqual(typeof a0.uid, 'number');
checkInvocations(a0, { before: 1, after: 1 }, 'when process exits');
});
2 changes: 1 addition & 1 deletion test/async-hooks/test-promise.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ function onexit() {
const a1 = as[1];
assert.strictEqual(a1.type, 'PROMISE', 'promise request');
assert.strictEqual(typeof a1.uid, 'number', 'uid is a number');
assert.strictEqual(a1.triggerId, 1, 'parent uid 1');
assert.strictEqual(a1.triggerId, a0.uid);
// We expect a destroy hook as well but we cannot guarentee predictable gc.
checkInvocations(a1, { init: 1, before: 1, after: 1 }, 'when process exits');
}
42 changes: 42 additions & 0 deletions test/async-hooks/test-promise.promise-before-init-hooks.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const initHooks = require('./init-hooks');
const { checkInvocations } = require('./hook-checks');

const p = new Promise(common.mustCall(function executor(resolve, reject) {
resolve(5);
}));

// init hooks after promise was created
const hooks = initHooks({allowNoInit: true});
hooks.enable();

p.then(function afterresolution(val) {
assert.strictEqual(val, 5);
const as = hooks.activitiesOfTypes('PROMISE');
assert.strictEqual(as.length, 1, 'one activity');
checkInvocations(as[0], { init: 1, before: 1 },
'after resolution child promise');
return val;
});

process.on('exit', function onexit() {
hooks.disable();
hooks.sanityCheck('PROMISE');

const as = hooks.activitiesOfTypes('PROMISE');
assert.strictEqual(as.length, 1);

const a0 = as[0];
assert.strictEqual(a0.type, 'PROMISE');
assert.strictEqual(typeof a0.uid, 'number');
// we can't get the asyncId from the parent dynamically, since init was
// never called. However, it is known that the parent promise was created
// immediately before the child promise, thus there should only be one
// difference in id.
assert.strictEqual(a0.triggerId, a0.uid - 1);
// We expect a destroy hook as well but we cannot guarentee predictable gc.
checkInvocations(a0, { init: 1, before: 1, after: 1 }, 'when process exits');
});