FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

async_hooks,inspector: implement inspector api without async_wrap · nodejs/node@4ddb9b3 · GitHub

/ node Public

Commit 4ddb9b3

Browse files
dygabo
authored and
targos
committed
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 <deb2001-github@yahoo.de> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
1 parent 2921d55 commit 4ddb9b3

6 files changed

Lines changed: 64 additions & 28 deletions

File tree

‎src/async_wrap.h‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -102,17 +102,9 @@ namespace node {
102102
#define NODE_ASYNC_CRYPTO_PROVIDER_TYPES(V)
103103
#endif // HAVE_OPENSSL
104104

105-
#if HAVE_INSPECTOR
106-
#define NODE_ASYNC_INSPECTOR_PROVIDER_TYPES(V) \
107-
V(INSPECTORJSBINDING)
108-
#else
109-
#define NODE_ASYNC_INSPECTOR_PROVIDER_TYPES(V)
110-
#endif // HAVE_INSPECTOR
111-
112-
#define NODE_ASYNC_PROVIDER_TYPES(V) \
113-
NODE_ASYNC_NON_CRYPTO_PROVIDER_TYPES(V) \
114-
NODE_ASYNC_CRYPTO_PROVIDER_TYPES(V) \
115-
NODE_ASYNC_INSPECTOR_PROVIDER_TYPES(V)
105+
#define NODE_ASYNC_PROVIDER_TYPES(V) \
106+
NODE_ASYNC_NON_CRYPTO_PROVIDER_TYPES(V) \
107+
NODE_ASYNC_CRYPTO_PROVIDER_TYPES(V)
116108

117109
class Environment;
118110
class DestroyParam;

‎src/inspector_js_api.cc‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
#include "async_wrap-inl.h"
21
#include "base_object-inl.h"
32
#include "inspector_agent.h"
43
#include "inspector_io.h"
@@ -61,7 +60,7 @@ struct MainThreadConnection {
6160
};
6261

6362
template <typename ConnectionType>
64-
class JSBindingsConnection : public AsyncWrap {
63+
class JSBindingsConnection : public BaseObject {
6564
public:
6665
class JSBindingsSessionDelegate : public InspectorSessionDelegate {
6766
public:
@@ -91,15 +90,16 @@ class JSBindingsConnection : public AsyncWrap {
9190
JSBindingsConnection(Environment* env,
9291
Local<Object> wrap,
9392
Local<Function> callback)
94-
: AsyncWrap(env, wrap, PROVIDER_INSPECTORJSBINDING),
95-
callback_(env->isolate(), callback) {
93+
: BaseObject(env, wrap), callback_(env->isolate(), callback) {
9694
Agent* inspector = env->inspector_agent();
9795
session_ = ConnectionType::Connect(
9896
inspector, std::make_unique<JSBindingsSessionDelegate>(env, this));
9997
}
10098

10199
void OnMessage(Local<Value> value) {
102-
MakeCallback(callback_.Get(env()->isolate()), 1, &value);
100+
auto result = callback_.Get(env()->isolate())
101+
->Call(env()->context(), object(), 1, &value);
102+
(void)result;
103103
}
104104

105105
static void Bind(Environment* env, Local<Object> target) {
@@ -108,7 +108,6 @@ class JSBindingsConnection : public AsyncWrap {
108108
NewFunctionTemplate(isolate, JSBindingsConnection::New);
109109
tmpl->InstanceTemplate()->SetInternalFieldCount(
110110
JSBindingsConnection::kInternalFieldCount);
111-
tmpl->Inherit(AsyncWrap::GetConstructorTemplate(env));
112111
SetProtoMethod(isolate, tmpl, "dispatch", JSBindingsConnection::Dispatch);
113112
SetProtoMethod(
114113
isolate, tmpl, "disconnect", JSBindingsConnection::Disconnect);

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

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
'use strict';
2+
const common = require('../common');
3+
const { AsyncLocalStorage } = require('async_hooks');
4+
const als = new AsyncLocalStorage();
5+
6+
function getStore() {
7+
return als.getStore();
8+
}
9+
10+
common.skipIfInspectorDisabled();
11+
12+
const assert = require('assert');
13+
const { Session } = require('inspector');
14+
const path = require('path');
15+
const { pathToFileURL } = require('url');
16+
17+
let valueInFunction = 0;
18+
let valueInBreakpoint = 0;
19+
20+
function debugged() {
21+
valueInFunction = getStore();
22+
return 42;
23+
}
24+
25+
async function test() {
26+
const session = new Session();
27+
28+
session.connect();
29+
session.post('Debugger.enable');
30+
31+
session.on('Debugger.paused', () => {
32+
valueInBreakpoint = getStore();
33+
});
34+
35+
await new Promise((resolve, reject) => {
36+
session.post('Debugger.setBreakpointByUrl', {
37+
'lineNumber': 22,
38+
'url': pathToFileURL(path.resolve(__dirname, __filename)).toString(),
39+
'columnNumber': 0,
40+
'condition': ''
41+
}, (error, result) => {
42+
return error ? reject(error) : resolve(result);
43+
});
44+
});
45+
46+
als.run(1, debugged);
47+
assert.strictEqual(valueInFunction, valueInBreakpoint);
48+
assert.strictEqual(valueInFunction, 1);
49+
50+
session.disconnect();
51+
}
52+
53+
const interval = setInterval(() => {}, 1000);
54+
test().then(common.mustCall(() => {
55+
clearInterval(interval);
56+
}));

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
// Flags: --expose-internals
21
'use strict';
32
const common = require('../common');
43

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

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,6 @@ const { getSystemErrorName } = require('util');
4747
delete providers.WORKER;
4848
// TODO(danbev): Test for these
4949
delete providers.JSUDPWRAP;
50-
if (!common.isMainThread)
51-
delete providers.INSPECTORJSBINDING;
5250
delete providers.KEYPAIRGENREQUEST;
5351
delete providers.KEYGENREQUEST;
5452
delete providers.KEYEXPORTREQUEST;
@@ -316,13 +314,6 @@ if (common.hasCrypto) { // eslint-disable-line node-core/crypto-check
316314
testInitialized(req, 'SendWrap');
317315
}
318316

319-
if (process.features.inspector && common.isMainThread) {
320-
const binding = internalBinding('inspector');
321-
const handle = new binding.Connection(() => {});
322-
testInitialized(handle, 'Connection');
323-
handle.disconnect();
324-
}
325-
326317
// PROVIDER_HEAPDUMP
327318
{
328319
v8.getHeapSnapshot().destroy();

‎typings/internalBinding/async_wrap.d.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ declare namespace InternalAsyncWrapBinding {
6868
SIGNREQUEST: 54;
6969
TLSWRAP: 55;
7070
VERIFYREQUEST: 56;
71-
INSPECTORJSBINDING: 57;
7271
}
7372
}
7473

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL