From e757a828ce5ce72a4e86c85ef7ce6e7332cf1974 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 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit :facepalm: Might help if I remember to disable the _other_ promise hook implementation when switching between them... Fixes #38814 Fixes #38815 Refs #36394 PR-URL: https://github.com/nodejs/node/pull/38912 Reviewed-By: Vladimir de Turckheim Reviewed-By: Gerhard Stöbich Reviewed-By: Bryan English --- lib/internal/async_hooks.js | 2 + ...ync-hooks-correctly-switch-promise-hook.js | 77 +++++++++++++++++++ 2 files changed, 79 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 37cee1e5a518ac..fbbf0225606728 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..73127f1ebaf94c --- /dev/null +++ b/test/parallel/test-async-hooks-correctly-switch-promise-hook.js @@ -0,0 +1,77 @@ +'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 layers = new Map(); + +// Only init to start context-based promise hook +async_hooks.createHook({ + init(asyncId, type) { + layers.set(asyncId, { + type, + init: true, + before: false, + after: false, + promiseResolve: false + }); + }, + before(asyncId) { + if (layers.has(asyncId)) { + layers.get(asyncId).before = true; + } + }, + after(asyncId) { + if (layers.has(asyncId)) { + layers.get(asyncId).after = true; + } + }, + promiseResolve(asyncId) { + if (layers.has(asyncId)) { + layers.get(asyncId).promiseResolve = true; + } + } +}).enable(); + +// With destroy, this should switch to native +// and disable context - based promise hook +async_hooks.createHook({ + init() { }, + destroy() { } +}).enable(); + +async function main() { + return Promise.resolve(); +} + +main(); + +process.on('exit', () => { + assert.deepStrictEqual(Array.from(layers.values()), [ + { + type: 'PROMISE', + init: true, + before: true, + after: true, + promiseResolve: true + }, + { + type: 'PROMISE', + init: true, + before: false, + after: false, + promiseResolve: true + }, + { + type: 'PROMISE', + init: true, + before: true, + after: true, + promiseResolve: true + }, + ]); +});