Repository: nodejs/node
Pull request: inspector: prevent propagation of promise hooks to noPromise hooks

<!--
Before submitting a pull request, please read:

- the CONTRIBUTING guide at https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md
- the commit message formatting guidelines at
  https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
2. Update documentation if relevant.
3. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

If you believe this PR should be highlighted in the Node.js CHANGELOG
please add the `notable-change` label.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->

This fix is intended to address a crash that occurs when inspecting code using async hooks with a debug build.

It resolves an issue where Promise hook events were incorrectly propagating to noPromiseHook.



### reproduction
The issue can be reproduced using the added test case.
Alternatively, you can reproduce it by debugging code that uses async hooks with Chrome Inspector.
Since it's caused by a debug check failure, it only occurs in debug builds.

The error message is as follows:
``` sh
#
# Fatal error in ../../deps/v8/src/inspector/v8-debugger.cc, line 1289
# Debug check failed: m_currentTasks.back() == task.
#
#
#
#FailureMessage Object: 0x16fdfd758
----- Native stack trace -----

 1: 0x10012d4f0 node::DumpNativeBacktrace(__sFILE*) [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
 2: 0x1003c9260 node::NodePlatform::GetStackTracePrinter()::$_0::operator()() const [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
 3: 0x1003c921c node::NodePlatform::GetStackTracePrinter()::$_0::__invoke() [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
 4: 0x102e74918 V8_Fatal(char const*, int, char const*, ...) [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
 5: 0x102e74288 v8::base::SetFatalFunction(void (*)(char const*, int, char const*)) [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
 6: 0x101543094 v8_inspector::V8Debugger::asyncTaskFinishedForStack(void*) [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
 7: 0x100569efc node::inspector::NodeInspectorClient::AsyncTaskFinished(void*) [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
 8: 0x100569ec0 node::inspector::Agent::AsyncTaskFinished(void*) [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
 9: 0x100597418 void node::inspector::(anonymous namespace)::InvokeAsyncTaskFnWithId<&node::inspector::Agent::AsyncTaskFinished(void*)>(v8::FunctionCallbackInfo<v8::Value> const&) [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
10: 0x101f76298 Builtins_CallApiCallbackGeneric [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
11: 0x101f7466c Builtins_InterpreterEntryTrampoline [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
12: 0x101f7466c Builtins_InterpreterEntryTrampoline [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
13: 0x101f7466c Builtins_InterpreterEntryTrampoline [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
14: 0x101fa6398 Builtins_RunMicrotasks [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
15: 0x101f71590 Builtins_JSRunMicrotasksEntry [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
16: 0x100a533b4 v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
17: 0x100a54bb4 v8::internal::(anonymous namespace)::InvokeWithTryCatch(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
18: 0x100a55244 v8::internal::Execution::TryRunMicrotasks(v8::internal::Isolate*, v8::internal::MicrotaskQueue*) [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
19: 0x100aaca68 v8::internal::MicrotaskQueue::RunMicrotasks(v8::internal::Isolate*) [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
20: 0x100aac718 v8::internal::MicrotaskQueue::PerformCheckpointInternal(v8::Isolate*) [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
21: 0x10004d124 node::InternalCallbackScope::Close() [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
22: 0x10004cf00 node::InternalCallbackScope::~InternalCallbackScope() [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
23: 0x10004c89c node::InternalCallbackScope::~InternalCallbackScope() [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
24: 0x1001f7d90 node::StartExecution(node::Environment*, std::__1::function<v8::MaybeLocal<v8::Value> (node::StartExecutionCallbackInfo const&)>) [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
25: 0x10005da38 node::LoadEnvironment(node::Environment*, std::__1::function<v8::MaybeLocal<v8::Value> (node::StartExecutionCallbackInfo const&)>, std::__1::function<void (node::Environment*, v8::Local<v8::Value>, v8::Local<v8::Value>)>) [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
26: 0x1003342d8 node::NodeMainInstance::Run(node::ExitCode*, node::Environment*) [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
27: 0x100333f64 node::NodeMainInstance::Run() [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
28: 0x1001fa9a4 node::StartInternal(int, char**) [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
29: 0x1001fa5c0 node::Start(int, char**) [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
30: 0x10258fb64 main [/Users/shimaryuuhei/workspace/islandryu2/node/out/Debug/node]
31: 0x1818a8274 start [/usr/lib/dyld]
```


Changed files:
filename: lib/internal/async_hooks.js
patch: @@ -189,7 +189,7 @@ function lookupPublicResource(resource) {
 // Used by C++ to call all init() callbacks. Because some state can be setup
 // from C++ there's no need to perform all the same operations as in
 // emitInitScript.
-function emitInitNative(asyncId, type, triggerAsyncId, resource) {
+function emitInitNative(asyncId, type, triggerAsyncId, resource, isPromiseHook) {
   active_hooks.call_depth += 1;
   resource = lookupPublicResource(resource);
   // Use a single try/catch for all hooks to avoid setting up one per iteration.
@@ -199,6 +199,10 @@ function emitInitNative(asyncId, type, triggerAsyncId, resource) {
     // eslint-disable-next-line no-var
     for (var i = 0; i < active_hooks.array.length; i++) {
       if (typeof active_hooks.array[i][init_symbol] === 'function') {
+        if (isPromiseHook &&
+            active_hooks.array[i][kNoPromiseHook]) {
+          continue;
+        }
         active_hooks.array[i][init_symbol](
           asyncId, type, triggerAsyncId,
           resource,
@@ -222,7 +226,7 @@ function emitInitNative(asyncId, type, triggerAsyncId, resource) {
 
 // Called from native. The asyncId stack handling is taken care of there
 // before this is called.
-function emitHook(symbol, asyncId) {
+function emitHook(symbol, asyncId, isPromiseHook) {
   active_hooks.call_depth += 1;
   // Use a single try/catch for all hook to avoid setting up one per
   // iteration.
@@ -232,6 +236,10 @@ function emitHook(symbol, asyncId) {
     // eslint-disable-next-line no-var
     for (var i = 0; i < active_hooks.array.length; i++) {
       if (typeof active_hooks.array[i][symbol] === 'function') {
+        if (isPromiseHook &&
+            active_hooks.array[i][kNoPromiseHook]) {
+          continue;
+        }
         active_hooks.array[i][symbol](asyncId);
       }
     }
@@ -321,7 +329,7 @@ function promiseInitHook(promise, parent) {
   trackPromise(promise, parent);
   const asyncId = promise[async_id_symbol];
   const triggerAsyncId = promise[trigger_async_id_symbol];
-  emitInitScript(asyncId, 'PROMISE', triggerAsyncId, promise);
+  emitInitScript(asyncId, 'PROMISE', triggerAsyncId, promise, true);
 }
 
 function promiseInitHookWithDestroyTracking(promise, parent) {
@@ -339,14 +347,14 @@ function promiseBeforeHook(promise) {
   trackPromise(promise);
   const asyncId = promise[async_id_symbol];
   const triggerId = promise[trigger_async_id_symbol];
-  emitBeforeScript(asyncId, triggerId, promise);
+  emitBeforeScript(asyncId, triggerId, promise, true);
 }
 
 function promiseAfterHook(promise) {
   trackPromise(promise);
   const asyncId = promise[async_id_symbol];
   if (hasHooks(kAfter)) {
-    emitAfterNative(asyncId);
+    emitAfterNative(asyncId, true);
   }
   if (asyncId === executionAsyncId()) {
     // This condition might not be true if async_hooks was enabled during
@@ -361,7 +369,7 @@ function promiseAfterHook(promise) {
 function promiseResolveHook(promise) {
   trackPromise(promise);
   const asyncId = promise[async_id_symbol];
-  emitPromiseResolveNative(asyncId);
+  emitPromiseResolveNative(asyncId, true);
 }
 
 let wantPromiseHook = false;
@@ -492,7 +500,7 @@ function promiseResolveHooksExist() {
 }
 
 
-function emitInitScript(asyncId, type, triggerAsyncId, resource) {
+function emitInitScript(asyncId, type, triggerAsyncId, resource, isPromiseHook = false) {
   // Short circuit all checks for the common case. Which is that no hooks have
   // been set. Do this to remove performance impact for embedders (and core).
   if (!hasHooks(kInit))
@@ -502,15 +510,15 @@ function emitInitScript(asyncId, type, triggerAsyncId, resource) {
     triggerAsyncId = getDefaultTriggerAsyncId();
   }
 
-  emitInitNative(asyncId, type, triggerAsyncId, resource);
+  emitInitNative(asyncId, type, triggerAsyncId, resource, isPromiseHook);
 }
 
 
-function emitBeforeScript(asyncId, triggerAsyncId, resource) {
+function emitBeforeScript(asyncId, triggerAsyncId, resource, isPromiseHook = false) {
   pushAsyncContext(asyncId, triggerAsyncId, resource);
 
   if (hasHooks(kBefore))
-    emitBeforeNative(asyncId);
+    emitBeforeNative(asyncId, isPromiseHook);
 }
 
 

filename: test/parallel/test-inspector-debug-async-hook.js
patch: @@ -0,0 +1,31 @@
+'use strict';
+const common = require('../common');
+common.skipIfInspectorDisabled();
+const test = require('node:test');
+const { NodeInstance } = require('../common/inspector-helper');
+
+const script = `
+import { createHook } from "async_hooks"
+import fs from "fs"
+
+const hook = createHook({
+    after() {
+    }
+});
+hook.enable(true);
+console.log('Async hook enabled');
+`;
+
+test('inspector async hooks should not crash in debug build', async () => {
+  const instance = new NodeInstance([
+    '--inspect-brk=0',
+  ], script);
+  const session = await instance.connectInspectorSession();
+  await session.send({ method: 'NodeRuntime.enable' });
+  await session.waitForNotification('NodeRuntime.waitingForDebugger');
+  await session.send({ method: 'Runtime.enable' });
+  await session.send({ method: 'Debugger.enable' });
+  await session.send({ id: 6, method: 'Debugger.setAsyncCallStackDepth', params: { maxDepth: 32 } });
+  await session.send({ method: 'Runtime.runIfWaitingForDebugger' });
+  await session.waitForDisconnect();
+});