Skip to content

Commit 135f4e6

Browse files
JiaLiPassionjasnell
authored andcommitted
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: #13302 PR-URL: #13367 Reviewed-By: Andreas Madsen <amwebdk@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent 9db02dc commit 135f4e6

4 files changed

+98
-1
lines changed

src/async-wrap.cc

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,23 @@ static void PromiseHook(PromiseHookType type, Local<Promise> promise,
294294
PromiseWrap* wrap = Unwrap<PromiseWrap>(promise);
295295
if (type == PromiseHookType::kInit || wrap == nullptr) {
296296
bool silent = type != PromiseHookType::kInit;
297+
// set parent promise's async Id as this promise's triggerId
298+
if (parent->IsPromise()) {
299+
// parent promise exists, current promise
300+
// is a chained promise, so we set parent promise's id as
301+
// current promise's triggerId
302+
Local<Promise> parent_promise = parent.As<Promise>();
303+
auto parent_wrap = Unwrap<PromiseWrap>(parent_promise);
304+
305+
if (parent_wrap == nullptr) {
306+
// create a new PromiseWrap for parent promise with silent parameter
307+
parent_wrap = new PromiseWrap(env, parent_promise, true);
308+
parent_wrap->MakeWeak(parent_wrap);
309+
}
310+
// get id from parentWrap
311+
double trigger_id = parent_wrap->get_id();
312+
env->set_init_trigger_id(trigger_id);
313+
}
297314
wrap = new PromiseWrap(env, promise, silent);
298315
wrap->MakeWeak(wrap);
299316
} else if (type == PromiseHookType::kResolve) {
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const initHooks = require('./init-hooks');
6+
const { checkInvocations } = require('./hook-checks');
7+
8+
const p = new Promise(common.mustCall(function executor(resolve, reject) {
9+
resolve(5);
10+
}));
11+
12+
p.then(function afterresolution(val) {
13+
assert.strictEqual(val, 5);
14+
return val;
15+
});
16+
17+
// init hooks after chained promise is created
18+
const hooks = initHooks();
19+
hooks._allowNoInit = true;
20+
hooks.enable();
21+
22+
23+
process.on('exit', function onexit() {
24+
hooks.disable();
25+
hooks.sanityCheck('PROMISE');
26+
27+
// Because the init event was never emitted the hooks listener doesn't
28+
// know what the type was. Thus check for Unknown rather than PROMISE.
29+
const as = hooks.activitiesOfTypes('PROMISE');
30+
const unknown = hooks.activitiesOfTypes('Unknown');
31+
assert.strictEqual(as.length, 0);
32+
assert.strictEqual(unknown.length, 1);
33+
34+
const a0 = unknown[0];
35+
assert.strictEqual(a0.type, 'Unknown');
36+
assert.strictEqual(typeof a0.uid, 'number');
37+
checkInvocations(a0, { before: 1, after: 1 }, 'when process exits');
38+
});

test/async-hooks/test-promise.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ function onexit() {
4646
const a1 = as[1];
4747
assert.strictEqual(a1.type, 'PROMISE', 'promise request');
4848
assert.strictEqual(typeof a1.uid, 'number', 'uid is a number');
49-
assert.strictEqual(a1.triggerId, 1, 'parent uid 1');
49+
assert.strictEqual(a1.triggerId, a0.uid);
5050
// We expect a destroy hook as well but we cannot guarentee predictable gc.
5151
checkInvocations(a1, { init: 1, before: 1, after: 1 }, 'when process exits');
5252
}
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const initHooks = require('./init-hooks');
6+
const { checkInvocations } = require('./hook-checks');
7+
8+
const p = new Promise(common.mustCall(function executor(resolve, reject) {
9+
resolve(5);
10+
}));
11+
12+
// init hooks after promise was created
13+
const hooks = initHooks({allowNoInit: true});
14+
hooks.enable();
15+
16+
p.then(function afterresolution(val) {
17+
assert.strictEqual(val, 5);
18+
const as = hooks.activitiesOfTypes('PROMISE');
19+
assert.strictEqual(as.length, 1, 'one activity');
20+
checkInvocations(as[0], { init: 1, before: 1 },
21+
'after resolution child promise');
22+
return val;
23+
});
24+
25+
process.on('exit', function onexit() {
26+
hooks.disable();
27+
hooks.sanityCheck('PROMISE');
28+
29+
const as = hooks.activitiesOfTypes('PROMISE');
30+
assert.strictEqual(as.length, 1);
31+
32+
const a0 = as[0];
33+
assert.strictEqual(a0.type, 'PROMISE');
34+
assert.strictEqual(typeof a0.uid, 'number');
35+
// we can't get the asyncId from the parent dynamically, since init was
36+
// never called. However, it is known that the parent promise was created
37+
// immediately before the child promise, thus there should only be one
38+
// difference in id.
39+
assert.strictEqual(a0.triggerId, a0.uid - 1);
40+
// We expect a destroy hook as well but we cannot guarentee predictable gc.
41+
checkInvocations(a0, { init: 1, before: 1, after: 1 }, 'when process exits');
42+
});

0 commit comments

Comments
 (0)