async_hooks,inspector: implement inspector api without async_wrap

Implementing the inspector session object as an async resource causes unwanted context change when a breakpoint callback function is being called. Modelling the inspector api without the AsyncWrap base class ensures that the callback has access to the AsyncLocalStorage instance that is active in the affected user function. See `test-inspector-async-context-brk.js` for an illustration of the use case. PR-URL: #51501 Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: Stephen Belanger <[email protected]>

Author: dygabo

src/async_wrap.h

@@ -102,17 +102,9 @@ namespace node{
 #defineNODE_ASYNC_CRYPTO_PROVIDER_TYPES(V)
 #endif// HAVE_OPENSSL
 
-#if HAVE_INSPECTOR
-#defineNODE_ASYNC_INSPECTOR_PROVIDER_TYPES(V) \
-V(INSPECTORJSBINDING)
-#else
-#defineNODE_ASYNC_INSPECTOR_PROVIDER_TYPES(V)
-#endif// HAVE_INSPECTOR
-
-#defineNODE_ASYNC_PROVIDER_TYPES(V) \
-NODE_ASYNC_NON_CRYPTO_PROVIDER_TYPES(V) \
-NODE_ASYNC_CRYPTO_PROVIDER_TYPES(V) \
-NODE_ASYNC_INSPECTOR_PROVIDER_TYPES(V)
+#defineNODE_ASYNC_PROVIDER_TYPES(V) \
+NODE_ASYNC_NON_CRYPTO_PROVIDER_TYPES(V) \
+NODE_ASYNC_CRYPTO_PROVIDER_TYPES(V)
 
 classEnvironment;
 classDestroyParam;

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>
-classJSBindingsConnection : publicAsyncWrap{
+classJSBindingsConnection : publicBaseObject{
 public:
 classJSBindingsSessionDelegate : publicInspectorSessionDelegate{
 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));
  }
 
 voidOnMessage(Local<Value> value){
-MakeCallback(callback_.Get(env()->isolate()), 1, &value);
+auto result = callback_.Get(env()->isolate())
+ ->Call(env()->context(), object(), 1, &value);
+ (void)result;
  }
 
 staticvoidBind(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';
+constcommon=require('../common');
+const{ AsyncLocalStorage }=require('async_hooks');
+constals=newAsyncLocalStorage();
+
+functiongetStore(){
+returnals.getStore();
+}
+
+common.skipIfInspectorDisabled();
+
+constassert=require('assert');
+const{ Session }=require('inspector');
+constpath=require('path');
+const{ pathToFileURL }=require('url');
+
+letvalueInFunction=0;
+letvalueInBreakpoint=0;
+
+functiondebugged(){
+valueInFunction=getStore();
+return42;
+}
+
+asyncfunctiontest(){
+constsession=newSession();
+
+session.connect();
+session.post('Debugger.enable');
+
+session.on('Debugger.paused',()=>{
+valueInBreakpoint=getStore();
+});
+
+awaitnewPromise((resolve,reject)=>{
+session.post('Debugger.setBreakpointByUrl',{
+'lineNumber': 22,
+'url': pathToFileURL(path.resolve(__dirname,__filename)).toString(),
+'columnNumber': 0,
+'condition': ''
+},(error,result)=>{
+returnerror ? reject(error) : resolve(result);
+});
+});
+
+als.run(1,debugged);
+assert.strictEqual(valueInFunction,valueInBreakpoint);
+assert.strictEqual(valueInFunction,1);
+
+session.disconnect();
+}
+
+constinterval=setInterval(()=>{},1000);
+test().then(common.mustCall(()=>{
+clearInterval(interval);
+}));

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

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

test/sequential/test-async-wrap-getasyncid.js

@@ -47,8 +47,6 @@ const{getSystemErrorName } = require('util');
 deleteproviders.WORKER;
 // TODO(danbev): Test for these
 deleteproviders.JSUDPWRAP;
-if(!common.isMainThread)
-deleteproviders.INSPECTORJSBINDING;
 deleteproviders.KEYPAIRGENREQUEST;
 deleteproviders.KEYGENREQUEST;
 deleteproviders.KEYEXPORTREQUEST;
@@ -316,13 +314,6 @@ if (common.hasCrypto){// eslint-disable-line node-core/crypto-check
 testInitialized(req,'SendWrap');
 }
 
-if(process.features.inspector&&common.isMainThread){
-constbinding=internalBinding('inspector');
-consthandle=newbinding.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;
 }
 }