Skip to content

Commit a268658

Browse files
eugeneoBridgeAR
authored andcommitted
inspector: new API - Session.connectToMainThread
This API is designed to enable worker threads use Inspector protocol on main thread (and other workers through NodeWorker domain). Note that worker can cause dead lock by suspending itself. I will work on a new API that will allow workers to be hidden from the inspector. Fixes: #28828 PR-URL: #28870 Reviewed-By: Aleksei Koziatinskii <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
1 parent bbf209b commit a268658

11 files changed

+282
-20
lines changed

doc/api/errors.md

+6
Original file line numberDiff line numberDiff line change
@@ -1213,6 +1213,12 @@ The `inspector` module is not available for use.
12131213
While using the `inspector` module, an attempt was made to use the inspector
12141214
before it was connected.
12151215

1216+
<a id="ERR_INSPECTOR_NOT_WORKER"></a>
1217+
### ERR_INSPECTOR_NOT_WORKER
1218+
1219+
An API was called on the main thread that can only be used from
1220+
the worker thread.
1221+
12161222
<a id="ERR_INVALID_ADDRESS_FAMILY"></a>
12171223
### ERR_INVALID_ADDRESS_FAMILY
12181224

doc/api/inspector.md

+9-3
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,15 @@ session.on('Debugger.paused', ({ params }) => {
121121
added: v8.0.0
122122
-->
123123

124-
Connects a session to the inspector back-end. An exception will be thrown
125-
if there is already a connected session established either through the API or by
126-
a front-end connected to the Inspector WebSocket port.
124+
Connects a session to the inspector back-end.
125+
126+
### session.connectToMainThread()
127+
<!-- YAML
128+
added: REPLACEME
129+
-->
130+
131+
Connects a session to the main thread inspector back-end. An exception will
132+
be thrown if this API was not called on a Worker thread.
127133

128134
### session.disconnect()
129135
<!-- YAML

lib/inspector.js

+14
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const {
99
ERR_INSPECTOR_NOT_AVAILABLE,
1010
ERR_INSPECTOR_NOT_CONNECTED,
1111
ERR_INSPECTOR_NOT_ACTIVE,
12+
ERR_INSPECTOR_NOT_WORKER,
1213
ERR_INVALID_ARG_TYPE,
1314
ERR_INVALID_CALLBACK
1415
} = require('internal/errors').codes;
@@ -20,8 +21,11 @@ if (!hasInspector)
2021
const EventEmitter = require('events');
2122
const { validateString } = require('internal/validators');
2223
const util = require('util');
24+
const { isMainThread } = require('worker_threads');
25+
2326
const {
2427
Connection,
28+
MainThreadConnection,
2529
open,
2630
url,
2731
waitForDebugger
@@ -47,6 +51,16 @@ class Session extends EventEmitter {
4751
new Connection((message) => this[onMessageSymbol](message));
4852
}
4953

54+
connectToMainThread() {
55+
if (isMainThread)
56+
throw new ERR_INSPECTOR_NOT_WORKER();
57+
if (this[connectionSymbol])
58+
throw new ERR_INSPECTOR_ALREADY_CONNECTED('The inspector session');
59+
this[connectionSymbol] =
60+
new MainThreadConnection(
61+
(message) => queueMicrotask(() => this[onMessageSymbol](message)));
62+
}
63+
5064
[onMessageSymbol](message) {
5165
const parsed = JSON.parse(message);
5266
try {

lib/internal/errors.js

+1
Original file line numberDiff line numberDiff line change
@@ -888,6 +888,7 @@ E('ERR_INSPECTOR_COMMAND', 'Inspector error %d: %s', Error);
888888
E('ERR_INSPECTOR_NOT_ACTIVE', 'Inspector is not active', Error);
889889
E('ERR_INSPECTOR_NOT_AVAILABLE', 'Inspector is not available', Error);
890890
E('ERR_INSPECTOR_NOT_CONNECTED', 'Session is not connected', Error);
891+
E('ERR_INSPECTOR_NOT_WORKER', 'Current thread is not a worker', Error);
891892
E('ERR_INTERNAL_ASSERTION', (message) => {
892893
const suffix = 'This is caused by either a bug in Node.js ' +
893894
'or incorrect usage of Node.js internals.\n' +

src/inspector/worker_inspector.cc

+6
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@ void ParentInspectorHandle::WorkerStarted(
7272
parent_thread_->Post(std::move(request));
7373
}
7474

75+
std::unique_ptr<inspector::InspectorSession> ParentInspectorHandle::Connect(
76+
std::unique_ptr<inspector::InspectorSessionDelegate> delegate,
77+
bool prevent_shutdown) {
78+
return parent_thread_->Connect(std::move(delegate), prevent_shutdown);
79+
}
80+
7581
void WorkerManager::WorkerFinished(int session_id) {
7682
children_.erase(session_id);
7783
}

src/inspector/worker_inspector.h

+5
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212

1313
namespace node {
1414
namespace inspector {
15+
class InspectorSession;
16+
class InspectorSessionDelegate;
1517
class MainThreadHandle;
1618
class WorkerManager;
1719

@@ -68,6 +70,9 @@ class ParentInspectorHandle {
6870
return wait_;
6971
}
7072
const std::string& url() const { return url_; }
73+
std::unique_ptr<inspector::InspectorSession> Connect(
74+
std::unique_ptr<inspector::InspectorSessionDelegate> delegate,
75+
bool prevent_shutdown);
7176

7277
private:
7378
int id_;

src/inspector_agent.cc

+11
Original file line numberDiff line numberDiff line change
@@ -828,6 +828,17 @@ std::unique_ptr<InspectorSession> Agent::Connect(
828828
new SameThreadInspectorSession(session_id, client_));
829829
}
830830

831+
std::unique_ptr<InspectorSession> Agent::ConnectToMainThread(
832+
std::unique_ptr<InspectorSessionDelegate> delegate,
833+
bool prevent_shutdown) {
834+
CHECK_NOT_NULL(parent_handle_);
835+
CHECK_NOT_NULL(client_);
836+
auto thread_safe_delegate =
837+
client_->getThreadHandle()->MakeDelegateThreadSafe(std::move(delegate));
838+
return parent_handle_->Connect(std::move(thread_safe_delegate),
839+
prevent_shutdown);
840+
}
841+
831842
void Agent::WaitForDisconnect() {
832843
CHECK_NOT_NULL(client_);
833844
bool is_worker = parent_handle_ != nullptr;

src/inspector_agent.h

+8-1
Original file line numberDiff line numberDiff line change
@@ -86,12 +86,19 @@ class Agent {
8686
std::unique_ptr<ParentInspectorHandle> GetParentHandle(
8787
int thread_id, const std::string& url);
8888

89-
// Called to create inspector sessions that can be used from the main thread.
89+
// Called to create inspector sessions that can be used from the same thread.
9090
// The inspector responds by using the delegate to send messages back.
9191
std::unique_ptr<InspectorSession> Connect(
9292
std::unique_ptr<InspectorSessionDelegate> delegate,
9393
bool prevent_shutdown);
9494

95+
// Called from the worker to create inspector sessions that is connected
96+
// to the main thread.
97+
// The inspector responds by using the delegate to send messages back.
98+
std::unique_ptr<InspectorSession> ConnectToMainThread(
99+
std::unique_ptr<InspectorSessionDelegate> delegate,
100+
bool prevent_shutdown);
101+
95102
void PauseOnNextJavascriptStatement(const std::string& reason);
96103

97104
std::string GetWsUrl() const;

src/inspector_js_api.cc

+42-14
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,29 @@ std::unique_ptr<StringBuffer> ToProtocolString(Isolate* isolate,
3737
return StringBuffer::create(StringView(*buffer, buffer.length()));
3838
}
3939

40+
struct LocalConnection {
41+
static std::unique_ptr<InspectorSession> Connect(
42+
Agent* inspector, std::unique_ptr<InspectorSessionDelegate> delegate) {
43+
return inspector->Connect(std::move(delegate), false);
44+
}
45+
46+
static Local<String> GetClassName(Environment* env) {
47+
return FIXED_ONE_BYTE_STRING(env->isolate(), "Connection");
48+
}
49+
};
50+
51+
struct MainThreadConnection {
52+
static std::unique_ptr<InspectorSession> Connect(
53+
Agent* inspector, std::unique_ptr<InspectorSessionDelegate> delegate) {
54+
return inspector->ConnectToMainThread(std::move(delegate), true);
55+
}
56+
57+
static Local<String> GetClassName(Environment* env) {
58+
return FIXED_ONE_BYTE_STRING(env->isolate(), "MainThreadConnection");
59+
}
60+
};
61+
62+
template <typename ConnectionType>
4063
class JSBindingsConnection : public AsyncWrap {
4164
public:
4265
class JSBindingsSessionDelegate : public InspectorSessionDelegate {
@@ -70,14 +93,29 @@ class JSBindingsConnection : public AsyncWrap {
7093
: AsyncWrap(env, wrap, PROVIDER_INSPECTORJSBINDING),
7194
callback_(env->isolate(), callback) {
7295
Agent* inspector = env->inspector_agent();
73-
session_ = inspector->Connect(std::make_unique<JSBindingsSessionDelegate>(
74-
env, this), false);
96+
session_ = ConnectionType::Connect(
97+
inspector, std::make_unique<JSBindingsSessionDelegate>(env, this));
7598
}
7699

77100
void OnMessage(Local<Value> value) {
78101
MakeCallback(callback_.Get(env()->isolate()), 1, &value);
79102
}
80103

104+
static void Bind(Environment* env, Local<Object> target) {
105+
Local<String> class_name = ConnectionType::GetClassName(env);
106+
Local<FunctionTemplate> tmpl =
107+
env->NewFunctionTemplate(JSBindingsConnection::New);
108+
tmpl->InstanceTemplate()->SetInternalFieldCount(1);
109+
tmpl->SetClassName(class_name);
110+
tmpl->Inherit(AsyncWrap::GetConstructorTemplate(env));
111+
env->SetProtoMethod(tmpl, "dispatch", JSBindingsConnection::Dispatch);
112+
env->SetProtoMethod(tmpl, "disconnect", JSBindingsConnection::Disconnect);
113+
target->Set(env->context(),
114+
class_name,
115+
tmpl->GetFunction(env->context()).ToLocalChecked())
116+
.ToChecked();
117+
}
118+
81119
static void New(const FunctionCallbackInfo<Value>& info) {
82120
Environment* env = Environment::GetCurrent(info);
83121
CHECK(info[0]->IsFunction());
@@ -300,18 +338,8 @@ void Initialize(Local<Object> target, Local<Value> unused,
300338
env->SetMethod(target, "registerAsyncHook", RegisterAsyncHookWrapper);
301339
env->SetMethodNoSideEffect(target, "isEnabled", IsEnabled);
302340

303-
auto conn_str = FIXED_ONE_BYTE_STRING(env->isolate(), "Connection");
304-
Local<FunctionTemplate> tmpl =
305-
env->NewFunctionTemplate(JSBindingsConnection::New);
306-
tmpl->InstanceTemplate()->SetInternalFieldCount(1);
307-
tmpl->SetClassName(conn_str);
308-
tmpl->Inherit(AsyncWrap::GetConstructorTemplate(env));
309-
env->SetProtoMethod(tmpl, "dispatch", JSBindingsConnection::Dispatch);
310-
env->SetProtoMethod(tmpl, "disconnect", JSBindingsConnection::Disconnect);
311-
target
312-
->Set(env->context(), conn_str,
313-
tmpl->GetFunction(env->context()).ToLocalChecked())
314-
.ToChecked();
341+
JSBindingsConnection<LocalConnection>::Bind(env, target);
342+
JSBindingsConnection<MainThreadConnection>::Bind(env, target);
315343
}
316344

317345
} // namespace

test/sequential/test-inspector-bindings.js test/parallel/test-inspector-bindings.js

+1-2
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
common.skipIfInspectorDisabled();
@@ -86,7 +85,7 @@ function testSampleDebugSession() {
8685
}, TypeError);
8786
session.post('Debugger.enable', () => cbAsSecondArgCalled = true);
8887
session.post('Debugger.setBreakpointByUrl', {
89-
'lineNumber': 14,
88+
'lineNumber': 13,
9089
'url': pathToFileURL(path.resolve(__dirname, __filename)).toString(),
9190
'columnNumber': 0,
9291
'condition': ''

0 commit comments

Comments
 (0)