From 9a2461e55a8efc59a96b30b3eeac832191f0d391 Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Wed, 2 Jun 2021 22:22:15 -0700 Subject: [PATCH] async_hooks: switch between native and context hooks correctly :facepalm: Might help if I remember to disable the _other_ promise hook implementation when switching between them... Fixes #38814 Fixes #38815 --- lib/internal/async_hooks.js | 2 + ...ync-hooks-correctly-switch-promise-hook.js | 61 +++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 test/parallel/test-async-hooks-correctly-switch-promise-hook.js diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js index eac2471ff79fb2..a6d258cf25757a 100644 --- a/lib/internal/async_hooks.js +++ b/lib/internal/async_hooks.js @@ -357,7 +357,9 @@ function updatePromiseHookMode() { wantPromiseHook = true; if (destroyHooksExist()) { enablePromiseHook(); + setPromiseHooks(undefined, undefined, undefined, undefined); } else { + disablePromiseHook(); setPromiseHooks( initHooksExist() ? promiseInitHook : undefined, promiseBeforeHook, diff --git a/test/parallel/test-async-hooks-correctly-switch-promise-hook.js b/test/parallel/test-async-hooks-correctly-switch-promise-hook.js new file mode 100644 index 00000000000000..54290686c60e82 --- /dev/null +++ b/test/parallel/test-async-hooks-correctly-switch-promise-hook.js @@ -0,0 +1,61 @@ +'use strict'; +require('../common'); +const assert = require('assert'); +const async_hooks = require('async_hooks'); + +// Regression test for: +// - https://github.com/nodejs/node/issues/38814 +// - https://github.com/nodejs/node/issues/38815 + +const expected = [ + ['init', 2, 'PROMISE', 1], + ['init', 3, 'PROMISE', 1], + ['promiseResolve', 3], + ['promiseResolve', 2], + ['before', 2], + ['init', 4, 'PROMISE', 3], + ['after', 2], + ['before', 4], + ['promiseResolve', 2], + ['promiseResolve', 4], + ['after', 4], +]; + +const actual = []; + +// Only init to start context-based promise hook +async_hooks.createHook({ + init() { } +}).enable(); + +// With destroy, this should switch to native +// and disable context - based promise hook +async_hooks.createHook({ + init(asyncId, type, triggerAsyncId) { + actual.push([ 'init', asyncId, type, triggerAsyncId ]); + }, + before(asyncId) { + actual.push([ 'before', asyncId ]); + }, + after(asyncId) { + actual.push([ 'after', asyncId ]); + }, + promiseResolve(asyncId) { + actual.push([ 'promiseResolve', asyncId ]); + }, + destroy(asyncId) { + actual.push([ 'destroy', asyncId ]); + } +}).enable(); + +async function main() { + return Promise.resolve(); +} + +main(); + +process.on('exit', () => { + for (const event of expected) { + assert.deepStrictEqual(actual.shift(), event); + } +});