From f44fce9a65cd02fd3c9ec698d0929bf07e38ec58 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 | 59 +++++++++++++++++++ 2 files changed, 61 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..dc0e29ce19aa8d --- /dev/null +++ b/test/parallel/test-async-hooks-correctly-switch-promise-hook.js @@ -0,0 +1,59 @@ +'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', () => { + assert.deepStrictEqual(actual, expected); +});