async_hooks,inspector: inspector keeps async context untouched

nodejs · Jan 17, 2024 · 29935b8 · 29935b8
1 parent e133e51
commit 29935b8
Show file tree

Hide file tree

Showing 5 changed files with 75 additions and 24 deletions.
diff --git a/src/async_wrap.h b/src/async_wrap.h
@@ -102,17 +102,9 @@ namespace node {
 #define NODE_ASYNC_CRYPTO_PROVIDER_TYPES(V)
 #endif  // HAVE_OPENSSL
 
-#if HAVE_INSPECTOR
-#define NODE_ASYNC_INSPECTOR_PROVIDER_TYPES(V)                                \
-  V(INSPECTORJSBINDING)
-#else
-#define NODE_ASYNC_INSPECTOR_PROVIDER_TYPES(V)
-#endif  // HAVE_INSPECTOR
-
 #define NODE_ASYNC_PROVIDER_TYPES(V)                                          \
   NODE_ASYNC_NON_CRYPTO_PROVIDER_TYPES(V)                                     \
-  NODE_ASYNC_CRYPTO_PROVIDER_TYPES(V)                                         \
-  NODE_ASYNC_INSPECTOR_PROVIDER_TYPES(V)
+  NODE_ASYNC_CRYPTO_PROVIDER_TYPES(V)
 
 class Environment;
 class DestroyParam;

diff --git a/src/inspector_js_api.cc b/src/inspector_js_api.cc
@@ -1,4 +1,3 @@
-#include "async_wrap-inl.h"
 #include "base_object-inl.h"
 #include "inspector_agent.h"
 #include "inspector_io.h"
@@ -61,7 +60,7 @@ struct MainThreadConnection {
 };
 
 template <typename ConnectionType>
-class JSBindingsConnection : public AsyncWrap {
+class JSBindingsConnection : public BaseObject {
  public:
   class JSBindingsSessionDelegate : public InspectorSessionDelegate {
    public:
@@ -91,15 +90,17 @@ class JSBindingsConnection : public AsyncWrap {
   JSBindingsConnection(Environment* env,
                        Local<Object> wrap,
                        Local<Function> callback)
-                       : AsyncWrap(env, wrap, PROVIDER_INSPECTORJSBINDING),
+                       : BaseObject(env, wrap),
                          callback_(env->isolate(), callback) {
     Agent* inspector = env->inspector_agent();
     session_ = ConnectionType::Connect(
         inspector, std::make_unique<JSBindingsSessionDelegate>(env, this));
   }
 
   void OnMessage(Local<Value> value) {
-    MakeCallback(callback_.Get(env()->isolate()), 1, &value);
+    auto result = callback_.Get(env()->isolate())
+        ->Call(env()->context(), object(), 1, &value);
+    (void)result;
   }
 
   static void Bind(Environment* env, Local<Object> target) {
@@ -108,7 +109,6 @@ class JSBindingsConnection : public AsyncWrap {
         NewFunctionTemplate(isolate, JSBindingsConnection::New);
     tmpl->InstanceTemplate()->SetInternalFieldCount(
         JSBindingsConnection::kInternalFieldCount);
-    tmpl->Inherit(AsyncWrap::GetConstructorTemplate(env));
     SetProtoMethod(isolate, tmpl, "dispatch", JSBindingsConnection::Dispatch);
     SetProtoMethod(
         isolate, tmpl, "disconnect", JSBindingsConnection::Disconnect);

diff --git a/test/parallel/test-inspector-async-context-brk.js b/test/parallel/test-inspector-async-context-brk.js
@@ -0,0 +1,69 @@
+// Flags: --expose-internals
+'use strict';
+const common = require('../common');
+const { AsyncLocalStorage } = require('async_hooks');
+const als = new AsyncLocalStorage();
+
+function getStore() {
+  return als.getStore();
+}
+
+common.skipIfInspectorDisabled();
+
+const assert = require('assert');
+const { Session } = require('inspector');
+const path = require('path');
+const { pathToFileURL } = require('url');
+
+let valueInFunction = 0;
+let valueInBreakpoint = 0;
+
+function debugged() {
+  valueInFunction = getStore();
+  console.log('in code => ', getStore());
+  return 42;
+}
+
+async function test() {
+  const session = new Session();
+
+  session.connect();
+  console.log('Connected');
+
+  session.post('Debugger.enable');
+  console.log('Debugger was enabled');
+
+  session.on('Debugger.paused', () => {
+    valueInBreakpoint = getStore();
+    console.log('on Debugger.paused callback => ', getStore());
+  });
+
+  await new Promise((resolve, reject) => {
+    session.post('Debugger.setBreakpointByUrl', {
+      'lineNumber': 22,
+      'url': pathToFileURL(path.resolve(__dirname, __filename)).toString(),
+      'columnNumber': 0,
+      'condition': ''
+    }, (error, result) => {
+      return error ? reject(error) : resolve(result);
+    });
+  });
+  console.log('Breakpoint was set');
+
+  als.run(1, debugged);
+  assert.strictEqual(valueInFunction, valueInBreakpoint);
+  assert.strictEqual(valueInFunction, 1);
+  assert.notStrictEqual(valueInFunction, 0);
+  assert.notStrictEqual(valueInBreakpoint, 0);
+
+  console.log('Breakpoint was hit');
+
+  session.disconnect();
+  console.log('Session disconnected');
+}
+
+const interval = setInterval(() => {}, 1000);
+test().then(common.mustCall(() => {
+  clearInterval(interval);
+  console.log('Done!');
+}));
diff --git a/test/sequential/test-async-wrap-getasyncid.js b/test/sequential/test-async-wrap-getasyncid.js
@@ -47,8 +47,6 @@ const { getSystemErrorName } = require('util');
     delete providers.WORKER;
     // TODO(danbev): Test for these
     delete providers.JSUDPWRAP;
-    if (!common.isMainThread)
-      delete providers.INSPECTORJSBINDING;
     delete providers.KEYPAIRGENREQUEST;
     delete providers.KEYGENREQUEST;
     delete providers.KEYEXPORTREQUEST;
@@ -316,13 +314,6 @@ if (common.hasCrypto) { // eslint-disable-line node-core/crypto-check
   testInitialized(req, 'SendWrap');
 }
 
-if (process.features.inspector && common.isMainThread) {
-  const binding = internalBinding('inspector');
-  const handle = new binding.Connection(() => {});
-  testInitialized(handle, 'Connection');
-  handle.disconnect();
-}
-
 // PROVIDER_HEAPDUMP
 {
   v8.getHeapSnapshot().destroy();

diff --git a/typings/internalBinding/async_wrap.d.ts b/typings/internalBinding/async_wrap.d.ts
@@ -68,7 +68,6 @@ declare namespace InternalAsyncWrapBinding {
     SIGNREQUEST: 54;
     TLSWRAP: 55;
     VERIFYREQUEST: 56;
-    INSPECTORJSBINDING: 57;
   }
 }