Commit 691c00c
async_hooks: switch between native and context hooks correctly
🤦 Might help if I remember to disable the _other_ promise
hook implementation when switching between them...
Fixes #38814
Fixes #38815
Refs #36394
PR-URL: #38912
Backport-PR-URL: #38577
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Bryan English <bryan@bryanenglish.com>1 parent e404841 commit 691c00c
2 files changed
Lines changed: 79 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -357,7 +357,9 @@ function updatePromiseHookMode() { | |||
| 357 | 357 | wantPromiseHook = true; | |
| 358 | 358 | if (destroyHooksExist()) { | |
| 359 | 359 | enablePromiseHook(); | |
| 360 | + setPromiseHooks(undefined, undefined, undefined, undefined); | ||
| 360 | 361 | } else { | |
| 362 | + disablePromiseHook(); | ||
| 361 | 363 | setPromiseHooks( | |
| 362 | 364 | initHooksExist() ? promiseInitHook : undefined, | |
| 363 | 365 | promiseBeforeHook, | |
Lines changed: 77 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,77 @@ | |||
| 1 | + 'use strict'; | ||
| 2 | + require('../common'); | ||
| 3 | + const assert = require('assert'); | ||
| 4 | + const async_hooks = require('async_hooks'); | ||
| 5 | + | ||
| 6 | + // Regression test for: | ||
| 7 | + // - https://github.com/nodejs/node/issues/38814 | ||
| 8 | + // - https://github.com/nodejs/node/issues/38815 | ||
| 9 | + | ||
| 10 | + const layers = new Map(); | ||
| 11 | + | ||
| 12 | + // Only init to start context-based promise hook | ||
| 13 | + async_hooks.createHook({ | ||
| 14 | + init(asyncId, type) { | ||
| 15 | + layers.set(asyncId, { | ||
| 16 | + type, | ||
| 17 | + init: true, | ||
| 18 | + before: false, | ||
| 19 | + after: false, | ||
| 20 | + promiseResolve: false | ||
| 21 | + }); | ||
| 22 | + }, | ||
| 23 | + before(asyncId) { | ||
| 24 | + if (layers.has(asyncId)) { | ||
| 25 | + layers.get(asyncId).before = true; | ||
| 26 | + } | ||
| 27 | + }, | ||
| 28 | + after(asyncId) { | ||
| 29 | + if (layers.has(asyncId)) { | ||
| 30 | + layers.get(asyncId).after = true; | ||
| 31 | + } | ||
| 32 | + }, | ||
| 33 | + promiseResolve(asyncId) { | ||
| 34 | + if (layers.has(asyncId)) { | ||
| 35 | + layers.get(asyncId).promiseResolve = true; | ||
| 36 | + } | ||
| 37 | + } | ||
| 38 | + }).enable(); | ||
| 39 | + | ||
| 40 | + // With destroy, this should switch to native | ||
| 41 | + // and disable context - based promise hook | ||
| 42 | + async_hooks.createHook({ | ||
| 43 | + init() { }, | ||
| 44 | + destroy() { } | ||
| 45 | + }).enable(); | ||
| 46 | + | ||
| 47 | + async function main() { | ||
| 48 | + return Promise.resolve(); | ||
| 49 | + } | ||
| 50 | + | ||
| 51 | + main(); | ||
| 52 | + | ||
| 53 | + process.on('exit', () => { | ||
| 54 | + assert.deepStrictEqual(Array.from(layers.values()), [ | ||
| 55 | + { | ||
| 56 | + type: 'PROMISE', | ||
| 57 | + init: true, | ||
| 58 | + before: true, | ||
| 59 | + after: true, | ||
| 60 | + promiseResolve: true | ||
| 61 | + }, | ||
| 62 | + { | ||
| 63 | + type: 'PROMISE', | ||
| 64 | + init: true, | ||
| 65 | + before: false, | ||
| 66 | + after: false, | ||
| 67 | + promiseResolve: true | ||
| 68 | + }, | ||
| 69 | + { | ||
| 70 | + type: 'PROMISE', | ||
| 71 | + init: true, | ||
| 72 | + before: true, | ||
| 73 | + after: true, | ||
| 74 | + promiseResolve: true | ||
| 75 | + }, | ||
| 76 | + ]); | ||
| 77 | + }); | ||
0 commit comments