Browse Source

async_hooks: use parent promise as triggerId

async_hooks init callback will be triggered when promise newly created,
in previous version, the parent promise which pass from chrome V8
PromiseHook is ignored, so we can't tell the promise is a pure
new promise or a chained promise.

In this commit, we use the parent promise's id as triggerId to
trigger the init callback.

Fixes: https://github.com/nodejs/node/issues/13302
PR-URL: https://github.com/nodejs/node/pull/13367
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
v6
JiaLi.Passion 7 years ago
committed by Anna Henningsen
parent
commit
8741e3c750
No known key found for this signature in database GPG Key ID: D8B9F5AEAE84E4CF
  1. 17
      src/async-wrap.cc
  2. 38
      test/async-hooks/test-promise.chain-promise-before-init-hooks.js
  3. 2
      test/async-hooks/test-promise.js
  4. 42
      test/async-hooks/test-promise.promise-before-init-hooks.js

17
src/async-wrap.cc

@ -294,6 +294,23 @@ static void PromiseHook(PromiseHookType type, Local<Promise> promise,
PromiseWrap* wrap = Unwrap<PromiseWrap>(promise); PromiseWrap* wrap = Unwrap<PromiseWrap>(promise);
if (type == PromiseHookType::kInit || wrap == nullptr) { if (type == PromiseHookType::kInit || wrap == nullptr) {
bool silent = type != PromiseHookType::kInit; 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 = new PromiseWrap(env, promise, silent);
wrap->MakeWeak(wrap); wrap->MakeWeak(wrap);
} else if (type == PromiseHookType::kResolve) { } else if (type == PromiseHookType::kResolve) {

38
test/async-hooks/test-promise.chain-promise-before-init-hooks.js

@ -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
test/async-hooks/test-promise.js

@ -46,7 +46,7 @@ function onexit() {
const a1 = as[1]; const a1 = as[1];
assert.strictEqual(a1.type, 'PROMISE', 'promise request'); assert.strictEqual(a1.type, 'PROMISE', 'promise request');
assert.strictEqual(typeof a1.uid, 'number', 'uid is a number'); 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. // We expect a destroy hook as well but we cannot guarentee predictable gc.
checkInvocations(a1, { init: 1, before: 1, after: 1 }, 'when process exits'); checkInvocations(a1, { init: 1, before: 1, after: 1 }, 'when process exits');
} }

42
test/async-hooks/test-promise.promise-before-init-hooks.js

@ -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');
});
Loading…
Cancel
Save