async_hooks,inspector: inspector keeps async context untouched

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)
+#define NODE_ASYNC_PROVIDER_TYPES(V)                                           \
+  NODE_ASYNC_NON_CRYPTO_PROVIDER_TYPES(V)                                      \
+  NODE_ASYNC_CRYPTO_PROVIDER_TYPES(V)
 
 class Environment;
 class DestroyParam;

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,16 @@ class JSBindingsConnection : public AsyncWrap {
   JSBindingsConnection(Environment* env,
                        Local<Object> wrap,
                        Local<Function> callback)
-                       : AsyncWrap(env, wrap, PROVIDER_INSPECTORJSBINDING),
-                         callback_(env->isolate(), callback) {
+      : 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 +108,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);

test/parallel/test-inspector-async-context-brk.js

@@ -0,0 +1,56 @@
+'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();
+  return 42;
+}
+
+async function test() {
+  const session = new Session();
+
+  session.connect();
+  session.post('Debugger.enable');
+
+  session.on('Debugger.paused', () => {
+    valueInBreakpoint = 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);
+    });
+  });
+
+  als.run(1, debugged);
+  assert.strictEqual(valueInFunction, valueInBreakpoint);
+  assert.strictEqual(valueInFunction, 1);
+
+  session.disconnect();
+}
+
+const interval = setInterval(() => {}, 1000);
+test().then(common.mustCall(() => {
+  clearInterval(interval);
+}));

test/parallel/test-inspector-multisession-js.js

@@ -1,4 +1,3 @@
-// Flags: --expose-internals
 'use strict';
 const common = require('../common');
 

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();

typings/internalBinding/async_wrap.d.ts

@@ -68,7 +68,6 @@ declare namespace InternalAsyncWrapBinding {
     SIGNREQUEST: 54;
     TLSWRAP: 55;
     VERIFYREQUEST: 56;
-    INSPECTORJSBINDING: 57;
   }
 }