Skip to content

Commit

Permalink
async_hooks: switch between native and context hooks correctly
Browse files Browse the repository at this point in the history
🤦 Might help if I remember to disable the _other_ promise
hook implementation when switching between them...

Fixes nodejs#38814
Fixes nodejs#38815
Refs nodejs#36394

PR-URL: nodejs#38912
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Bryan English <bryan@bryanenglish.com>
  • Loading branch information
Stephen Belanger committed Jul 21, 2021
1 parent 7eb14e1 commit 7969199
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 0 deletions.
2 changes: 2 additions & 0 deletions lib/internal/async_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,9 @@ function updatePromiseHookMode() {
wantPromiseHook = true;
if (destroyHooksExist()) {
enablePromiseHook();
setPromiseHooks(undefined, undefined, undefined, undefined);
} else {
disablePromiseHook();
setPromiseHooks(
initHooksExist() ? promiseInitHook : undefined,
promiseBeforeHook,
Expand Down
77 changes: 77 additions & 0 deletions test/parallel/test-async-hooks-correctly-switch-promise-hook.js
Original file line number Diff line number Diff line change
@@ -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
},
]);
});

0 comments on commit 7969199

Please sign in to comment.