inspector: prevent propagation of promise hooks to noPromise hooks

PR-URL: #58841
Reviewed-By: Chengzhong Wu <[email protected]>

Author: islandryu

lib/internal/async_hooks.js

@@ -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);
 }
 
 

test/parallel/test-inspector-debug-async-hook.js

@@ -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();
+});