From 64a2c4ba947868dd0db4cdd1624a28ced2865cab Mon Sep 17 00:00:00 2001 From: Maya Lekova Date: Wed, 9 May 2018 15:05:51 +0200 Subject: [PATCH 1/4] async_hooks,test: add test for async hooks parity for async/await Add a basic test ensuring parity between before-after and init-promiseResolve hooks when using async/await. Refs: https://github.com/nodejs/node/issues/20516 --- test/async-hooks/test-async-await.js | 80 ++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 test/async-hooks/test-async-await.js diff --git a/test/async-hooks/test-async-await.js b/test/async-hooks/test-async-await.js new file mode 100644 index 00000000000000..2b48428d82a053 --- /dev/null +++ b/test/async-hooks/test-async-await.js @@ -0,0 +1,80 @@ +'use strict'; +const common = require('../common'); + +// This test ensures async hooks are being properly called +// when using async-await mechanics. This involves: +// 1. Checking that all initialized promises are being resolved +// 2. Checking that for each 'before' corresponding hook 'after' hook is called + +const assert = require('assert'); +const asyncHooks = require('async_hooks'); +const util = require('util'); + +const sleep = util.promisify(setTimeout); +const promiseCallbacks = new Map(); +const resolvedPromises = new Set(); + +const asyncHook = asyncHooks.createHook({ + init, before, after, promiseResolve +}); +asyncHook.enable(); + +function init(asyncId, type, triggerAsyncId, resource) { + if (type === 'PROMISE') { + promiseCallbacks.set(asyncId, 0); + } +} + +function before(asyncId) { + if (promiseCallbacks.has(asyncId)) { + assert.strictEqual(promiseCallbacks.get(asyncId), 0, + 'before hook called for promise without prior call' + + 'to init hook'); + promiseCallbacks.set(asyncId, 1); + } +} + +function after(asyncId) { + if (promiseCallbacks.has(asyncId)) { + assert.strictEqual(promiseCallbacks.get(asyncId), 1, + 'after hook called for promise without prior call' + + 'to before hook'); + promiseCallbacks.set(asyncId, 0); + } +} + +function promiseResolve(asyncId) { + assert(promiseCallbacks.has(asyncId), + 'resolve hook called for promise without prior call to init hook'); + + resolvedPromises.add(asyncId); +} + +const timeout = common.platformTimeout(10); + +function checkPromiseCallbacks() { + for (const balance of promiseCallbacks.values()) { + assert.strictEqual(balance, 0, + 'mismatch between before and after hook calls'); + } +} + +function checkPromiseResolution() { + for (const id of promiseCallbacks.keys()) { + assert(resolvedPromises.has(id), + 'promise initialized without being resolved'); + } +} + +process.on('beforeExit', common.mustCall(() => { + asyncHook.disable(); + + checkPromiseResolution(); + checkPromiseCallbacks(); +})); + +async function asyncFunc(callback) { + await sleep(timeout); +} + +asyncFunc(); From 35b8fcf568162dfb8e5a393c0d8f10d0c4c70c80 Mon Sep 17 00:00:00 2001 From: Maya Lekova Date: Fri, 11 May 2018 11:44:45 +0200 Subject: [PATCH 2/4] async_hooks,test: enhance initHooks with promiseResolve hook Add ability to initHooks and to checkInvocations utilities to transmit promiseResolve hook as well. Refs: #20516 --- test/async-hooks/hook-checks.js | 2 +- test/async-hooks/init-hooks.js | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/test/async-hooks/hook-checks.js b/test/async-hooks/hook-checks.js index 2abed61555a158..44970378131c4a 100644 --- a/test/async-hooks/hook-checks.js +++ b/test/async-hooks/hook-checks.js @@ -25,7 +25,7 @@ exports.checkInvocations = function checkInvocations(activity, hooks, stage) { ); // Check that actual invocations for all hooks match the expected invocations - [ 'init', 'before', 'after', 'destroy' ].forEach(checkHook); + [ 'init', 'before', 'after', 'destroy', 'promiseResolve' ].forEach(checkHook); function checkHook(k) { const val = hooks[k]; diff --git a/test/async-hooks/init-hooks.js b/test/async-hooks/init-hooks.js index e43761df623dc4..509f443b29a671 100644 --- a/test/async-hooks/init-hooks.js +++ b/test/async-hooks/init-hooks.js @@ -25,6 +25,7 @@ class ActivityCollector { onbefore, onafter, ondestroy, + onpromiseResolve, logid = null, logtype = null } = {}) { @@ -39,13 +40,16 @@ class ActivityCollector { this.onbefore = typeof onbefore === 'function' ? onbefore : noop; this.onafter = typeof onafter === 'function' ? onafter : noop; this.ondestroy = typeof ondestroy === 'function' ? ondestroy : noop; + this.onpromiseResolve = typeof onpromiseResolve === 'function' ? + onpromiseResolve : noop; // Create the hook with which we'll collect activity data this._asyncHook = async_hooks.createHook({ init: this._init.bind(this), before: this._before.bind(this), after: this._after.bind(this), - destroy: this._destroy.bind(this) + destroy: this._destroy.bind(this), + promiseResolve: this._promiseResolve.bind(this) }); } @@ -206,6 +210,13 @@ class ActivityCollector { this.ondestroy(uid); } + _promiseResolve(uid) { + const h = this._getActivity(uid, 'promiseResolve'); + this._stamp(h, 'promiseResolve'); + this._maybeLog(uid, h && h.type, 'promiseResolve'); + this.onpromiseResolve(uid); + } + _maybeLog(uid, type, name) { if (this._logid && (type == null || this._logtype == null || this._logtype === type)) { @@ -219,6 +230,7 @@ exports = module.exports = function initHooks({ onbefore, onafter, ondestroy, + onpromiseResolve, allowNoInit, logid, logtype @@ -228,6 +240,7 @@ exports = module.exports = function initHooks({ onbefore, onafter, ondestroy, + onpromiseResolve, allowNoInit, logid, logtype From dc98e8049630b6d897316b0e929f2b56bb8d662a Mon Sep 17 00:00:00 2001 From: Maya Lekova Date: Fri, 11 May 2018 11:47:36 +0200 Subject: [PATCH 3/4] async_hooks,test: refactor async/await test to use initHooks Reword async/await test to make use of the initHooks utility and an attempt to clarify the test's logic. Refs: #20516 --- test/async-hooks/test-async-await.js | 80 ++++++++++++++++------------ 1 file changed, 45 insertions(+), 35 deletions(-) diff --git a/test/async-hooks/test-async-await.js b/test/async-hooks/test-async-await.js index 2b48428d82a053..d742a115459c90 100644 --- a/test/async-hooks/test-async-await.js +++ b/test/async-hooks/test-async-await.js @@ -7,70 +7,80 @@ const common = require('../common'); // 2. Checking that for each 'before' corresponding hook 'after' hook is called const assert = require('assert'); -const asyncHooks = require('async_hooks'); +const initHooks = require('./init-hooks'); + const util = require('util'); const sleep = util.promisify(setTimeout); -const promiseCallbacks = new Map(); -const resolvedPromises = new Set(); - -const asyncHook = asyncHooks.createHook({ - init, before, after, promiseResolve +// either 'inited' or 'resolved' +const promisesInitState = new Map(); +// either 'before' or 'after' AND asyncId must be present in the other map +const promisesExecutionState = new Map(); + +const hooks = initHooks({ + oninit, + onbefore, + onafter, + ondestroy: null, // Intentionally not tested, since it will be removed soon + onpromiseResolve }); -asyncHook.enable(); +hooks.enable(); -function init(asyncId, type, triggerAsyncId, resource) { +function oninit(asyncId, type, triggerAsyncId, resource) { if (type === 'PROMISE') { - promiseCallbacks.set(asyncId, 0); + promisesInitState.set(asyncId, 'inited'); } } -function before(asyncId) { - if (promiseCallbacks.has(asyncId)) { - assert.strictEqual(promiseCallbacks.get(asyncId), 0, - 'before hook called for promise without prior call' + - 'to init hook'); - promiseCallbacks.set(asyncId, 1); +function onbefore(asyncId) { + if (!promisesInitState.has(asyncId)) { + return; } + promisesExecutionState.set(asyncId, 'before'); } -function after(asyncId) { - if (promiseCallbacks.has(asyncId)) { - assert.strictEqual(promiseCallbacks.get(asyncId), 1, - 'after hook called for promise without prior call' + - 'to before hook'); - promiseCallbacks.set(asyncId, 0); +function onafter(asyncId) { + if (!promisesInitState.has(asyncId)) { + return; } + + assert.strictEqual(promisesExecutionState.get(asyncId), 'before', + 'after hook called for promise without prior call' + + 'to before hook'); + assert.strictEqual(promisesInitState.get(asyncId), 'resolved', + 'after hook called for promise without prior call' + + 'to resolve hook'); + promisesExecutionState.set(asyncId, 'after'); } -function promiseResolve(asyncId) { - assert(promiseCallbacks.has(asyncId), +function onpromiseResolve(asyncId) { + assert(promisesInitState.has(asyncId), 'resolve hook called for promise without prior call to init hook'); - resolvedPromises.add(asyncId); + promisesInitState.set(asyncId, 'resolved'); } const timeout = common.platformTimeout(10); -function checkPromiseCallbacks() { - for (const balance of promiseCallbacks.values()) { - assert.strictEqual(balance, 0, - 'mismatch between before and after hook calls'); +function checkPromisesInitState() { + for (const initState of promisesInitState.values()) { + assert.strictEqual(initState, 'resolved', + 'promise initialized without being resolved'); } } -function checkPromiseResolution() { - for (const id of promiseCallbacks.keys()) { - assert(resolvedPromises.has(id), - 'promise initialized without being resolved'); +function checkPromisesExecutionState() { + for (const executionState of promisesExecutionState.values()) { + assert.strictEqual(executionState, 'after', + 'mismatch between before and after hook calls'); } } process.on('beforeExit', common.mustCall(() => { - asyncHook.disable(); + hooks.disable(); - checkPromiseResolution(); - checkPromiseCallbacks(); + checkPromisesInitState(); + checkPromisesExecutionState(); })); async function asyncFunc(callback) { From d70af92b556badc5fc4064187843d2a3e1fc4d2a Mon Sep 17 00:00:00 2001 From: Maya Lekova Date: Mon, 14 May 2018 08:51:41 +0200 Subject: [PATCH 4/4] async_hooks,test: remove unused param from async/await test Refs: #20516 --- test/async-hooks/test-async-await.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/async-hooks/test-async-await.js b/test/async-hooks/test-async-await.js index d742a115459c90..7f88cd9b18176f 100644 --- a/test/async-hooks/test-async-await.js +++ b/test/async-hooks/test-async-await.js @@ -78,12 +78,13 @@ function checkPromisesExecutionState() { process.on('beforeExit', common.mustCall(() => { hooks.disable(); + hooks.sanityCheck('PROMISE'); checkPromisesInitState(); checkPromisesExecutionState(); })); -async function asyncFunc(callback) { +async function asyncFunc() { await sleep(timeout); }