inspector: fix disable async hooks on Debugger.setAsyncCallStackDepth

This was previously calling the enable function by mistake. As a
result, when profiling using Chrome DevTools, the async hooks won't
be turned off properly after receiving Debugger.setAsyncCallStackDepth
with depth 0.

PR-URL: #53473
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>

Author: joyeecheung

src/async_wrap.cc

@@ -193,6 +193,12 @@ static void SetPromiseHooks(const FunctionCallbackInfo<Value>& args) {
       args[3]->IsFunction() ? args[3].As<Function>() : Local<Function>());
 }
 
+static void GetPromiseHooks(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+  args.GetReturnValue().Set(
+      env->async_hooks()->GetPromiseHooks(args.GetIsolate()));
+}
+
 class DestroyParam {
  public:
   double asyncId;
@@ -364,6 +370,7 @@ void AsyncWrap::CreatePerIsolateProperties(IsolateData* isolate_data,
   SetMethod(isolate, target, "clearAsyncIdStack", ClearAsyncIdStack);
   SetMethod(isolate, target, "queueDestroyAsyncId", QueueDestroyAsyncId);
   SetMethod(isolate, target, "setPromiseHooks", SetPromiseHooks);
+  SetMethod(isolate, target, "getPromiseHooks", GetPromiseHooks);
   SetMethod(isolate, target, "registerDestroyHook", RegisterDestroyHook);
   AsyncWrap::GetConstructorTemplate(isolate_data);
 }
@@ -469,6 +476,7 @@ void AsyncWrap::RegisterExternalReferences(
   registry->Register(ClearAsyncIdStack);
   registry->Register(QueueDestroyAsyncId);
   registry->Register(SetPromiseHooks);
+  registry->Register(GetPromiseHooks);
   registry->Register(RegisterDestroyHook);
   registry->Register(AsyncWrap::GetAsyncId);
   registry->Register(AsyncWrap::AsyncReset);

src/env.cc

@@ -87,6 +87,18 @@ void AsyncHooks::ResetPromiseHooks(Local<Function> init,
   js_promise_hooks_[3].Reset(env()->isolate(), resolve);
 }
 
+Local<Array> AsyncHooks::GetPromiseHooks(Isolate* isolate) {
+  std::vector<Local<Value>> values;
+  for (size_t i = 0; i < js_promise_hooks_.size(); ++i) {
+    if (js_promise_hooks_[i].IsEmpty()) {
+      values.push_back(Undefined(isolate));
+    } else {
+      values.push_back(js_promise_hooks_[i].Get(isolate));
+    }
+  }
+  return Array::New(isolate, values.data(), values.size());
+}
+
 void Environment::ResetPromiseHooks(Local<Function> init,
                                     Local<Function> before,
                                     Local<Function> after,

src/env.h

@@ -322,7 +322,9 @@ class AsyncHooks : public MemoryRetainer {
                          v8::Local<v8::Function> before,
                          v8::Local<v8::Function> after,
                          v8::Local<v8::Function> resolve);
-
+  // Used for testing since V8 doesn't provide API for retrieving configured
+  // JS promise hooks.
+  v8::Local<v8::Array> GetPromiseHooks(v8::Isolate* isolate);
   inline v8::Local<v8::String> provider_string(int idx);
 
   inline void no_force_checks();

src/inspector_agent.cc

@@ -923,7 +923,7 @@ void Agent::EnableAsyncHook() {
 
 void Agent::DisableAsyncHook() {
   HandleScope scope(parent_env_->isolate());
-  Local<Function> disable = parent_env_->inspector_enable_async_hooks();
+  Local<Function> disable = parent_env_->inspector_disable_async_hooks();
   if (!disable.IsEmpty()) {
     ToggleAsyncHook(parent_env_->isolate(), disable);
   } else if (pending_enable_async_hook_) {

test/parallel/test-inspector-async-call-stack.js

@@ -5,21 +5,32 @@ common.skipIfInspectorDisabled();
 common.skipIf32Bits();
 
 const assert = require('assert');
+const { inspect } = require('util');
 const { internalBinding } = require('internal/test/binding');
-const { async_hook_fields, constants } = internalBinding('async_wrap');
+const { async_hook_fields, constants, getPromiseHooks } = internalBinding('async_wrap');
 const { kTotals } = constants;
-const inspector = require('inspector');
+const inspector = require('inspector/promises');
 
 const setDepth = 'Debugger.setAsyncCallStackDepth';
-
+const emptyPromiseHooks = [ undefined, undefined, undefined, undefined ];
 function verifyAsyncHookDisabled(message) {
   assert.strictEqual(async_hook_fields[kTotals], 0,
                      `${async_hook_fields[kTotals]} !== 0: ${message}`);
+  const promiseHooks = getPromiseHooks();
+  assert.deepStrictEqual(
+    promiseHooks, emptyPromiseHooks,
+    `${message}: promise hooks ${inspect(promiseHooks)}`
+  );
 }
 
 function verifyAsyncHookEnabled(message) {
   assert.strictEqual(async_hook_fields[kTotals], 4,
                      `${async_hook_fields[kTotals]} !== 4: ${message}`);
+  const promiseHooks = getPromiseHooks();
+  assert.notDeepStrictEqual(
+    promiseHooks, emptyPromiseHooks,
+    `${message}: promise hooks ${inspect(promiseHooks)}`
+  );
 }
 
 // By default inspector async hooks should not have been installed.
@@ -31,53 +42,38 @@ verifyAsyncHookDisabled('creating a session should not enable async hooks');
 session.connect();
 verifyAsyncHookDisabled('connecting a session should not enable async hooks');
 
-session.post('Debugger.enable', () => {
+(async () => {
+  await session.post('Debugger.enable');
   verifyAsyncHookDisabled('enabling debugger should not enable async hooks');
-
-  session.post(setDepth, { invalid: 'message' }, () => {
-    verifyAsyncHookDisabled('invalid message should not enable async hooks');
-
-    session.post(setDepth, { maxDepth: 'five' }, () => {
-      verifyAsyncHookDisabled('invalid maxDepth (string) should not enable ' +
-                              'async hooks');
-
-      session.post(setDepth, { maxDepth: NaN }, () => {
-        verifyAsyncHookDisabled('invalid maxDepth (NaN) should not enable ' +
-                                'async hooks');
-
-        session.post(setDepth, { maxDepth: 10 }, () => {
-          verifyAsyncHookEnabled('valid message should enable async hooks');
-
-          session.post(setDepth, { maxDepth: 0 }, () => {
-            verifyAsyncHookDisabled('Setting maxDepth to 0 should disable ' +
-                                    'async hooks');
-
-            runTestSet2(session);
-          });
-        });
-      });
-    });
-  });
-});
-
-function runTestSet2(session) {
-  session.post(setDepth, { maxDepth: 32 }, () => {
-    verifyAsyncHookEnabled('valid message should enable async hooks');
-
-    session.post('Debugger.disable', () => {
-      verifyAsyncHookDisabled('Debugger.disable should disable async hooks');
-
-      session.post('Debugger.enable', () => {
-        verifyAsyncHookDisabled('Enabling debugger should not enable hooks');
-
-        session.post(setDepth, { maxDepth: 64 }, () => {
-          verifyAsyncHookEnabled('valid message should enable async hooks');
-
-          session.disconnect();
-          verifyAsyncHookDisabled('Disconnecting session should disable ' +
-                                  'async hooks');
-        });
-      });
-    });
-  });
-}
+  await assert.rejects(session.post(setDepth, { invalid: 'message' }), { code: 'ERR_INSPECTOR_COMMAND' });
+  verifyAsyncHookDisabled('invalid message should not enable async hooks');
+  await assert.rejects(session.post(setDepth, { maxDepth: 'five' }), { code: 'ERR_INSPECTOR_COMMAND' });
+  verifyAsyncHookDisabled('invalid maxDepth (string) should not enable ' +
+                          'async hooks');
+  await assert.rejects(session.post(setDepth, { maxDepth: NaN }), { code: 'ERR_INSPECTOR_COMMAND' });
+  verifyAsyncHookDisabled('invalid maxDepth (NaN) should not enable ' +
+                          'async hooks');
+  await session.post(setDepth, { maxDepth: 10 });
+  verifyAsyncHookEnabled('valid message should enable async hooks');
+
+  await session.post(setDepth, { maxDepth: 0 });
+  verifyAsyncHookDisabled('Setting maxDepth to 0 should disable ' +
+                          'async hooks');
+
+  await session.post(setDepth, { maxDepth: 32 });
+  verifyAsyncHookEnabled('valid message should enable async hooks');
+
+  await session.post('Debugger.disable');
+  verifyAsyncHookDisabled('Debugger.disable should disable async hooks');
+
+  await session.post('Debugger.enable');
+  verifyAsyncHookDisabled('Enabling debugger should not enable hooks');
+
+  await session.post(setDepth, { maxDepth: 64 });
+  verifyAsyncHookEnabled('valid message should enable async hooks');
+
+  await session.disconnect();
+
+  verifyAsyncHookDisabled('Disconnecting session should disable ' +
+                          'async hooks');
+})().then(common.mustCall());