Skip to content

Commit

Permalink
inspector: new API - Session.connectToMainThread
Browse files Browse the repository at this point in the history
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: nodejs#28828
PR-URL: nodejs#28870
Reviewed-By: Aleksei Koziatinskii <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
  • Loading branch information
eugeneo committed Sep 16, 2019
1 parent 70abb4f commit 3d841fe
Showing 11 changed files with 282 additions and 20 deletions.
6 changes: 6 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
@@ -1215,6 +1215,12 @@ The `inspector` module is not available for use.
While using the `inspector` module, an attempt was made to use the inspector
before it was connected.

<a id="ERR_INSPECTOR_NOT_WORKER"></a>
### ERR_INSPECTOR_NOT_WORKER

An API was called on the main thread that can only be used from
the worker thread.

<a id="ERR_INVALID_ADDRESS_FAMILY"></a>
### ERR_INVALID_ADDRESS_FAMILY

12 changes: 9 additions & 3 deletions doc/api/inspector.md
Original file line number Diff line number Diff line change
@@ -121,9 +121,15 @@ session.on('Debugger.paused', ({ params }) => {
added: v8.0.0
-->

Connects a session to the inspector back-end. An exception will be thrown
if there is already a connected session established either through the API or by
a front-end connected to the Inspector WebSocket port.
Connects a session to the inspector back-end.

### session.connectToMainThread()
<!-- YAML
added: REPLACEME
-->

Connects a session to the main thread inspector back-end. An exception will
be thrown if this API was not called on a Worker thread.

### session.disconnect()
<!-- YAML
14 changes: 14 additions & 0 deletions lib/inspector.js
Original file line number Diff line number Diff line change
@@ -9,6 +9,7 @@ const {
ERR_INSPECTOR_NOT_AVAILABLE,
ERR_INSPECTOR_NOT_CONNECTED,
ERR_INSPECTOR_NOT_ACTIVE,
ERR_INSPECTOR_NOT_WORKER,
ERR_INVALID_ARG_TYPE,
ERR_INVALID_CALLBACK
} = require('internal/errors').codes;
@@ -20,8 +21,11 @@ if (!hasInspector)
const EventEmitter = require('events');
const { validateString } = require('internal/validators');
const util = require('util');
const { isMainThread } = require('worker_threads');

const {
Connection,
MainThreadConnection,
open,
url,
waitForDebugger
@@ -47,6 +51,16 @@ class Session extends EventEmitter {
new Connection((message) => this[onMessageSymbol](message));
}

connectToMainThread() {
if (isMainThread)
throw new ERR_INSPECTOR_NOT_WORKER();
if (this[connectionSymbol])
throw new ERR_INSPECTOR_ALREADY_CONNECTED('The inspector session');
this[connectionSymbol] =
new MainThreadConnection(
(message) => queueMicrotask(() => this[onMessageSymbol](message)));
}

[onMessageSymbol](message) {
const parsed = JSON.parse(message);
try {
1 change: 1 addition & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
@@ -893,6 +893,7 @@ E('ERR_INSPECTOR_COMMAND', 'Inspector error %d: %s', Error);
E('ERR_INSPECTOR_NOT_ACTIVE', 'Inspector is not active', Error);
E('ERR_INSPECTOR_NOT_AVAILABLE', 'Inspector is not available', Error);
E('ERR_INSPECTOR_NOT_CONNECTED', 'Session is not connected', Error);
E('ERR_INSPECTOR_NOT_WORKER', 'Current thread is not a worker', Error);
E('ERR_INTERNAL_ASSERTION', (message) => {
const suffix = 'This is caused by either a bug in Node.js ' +
'or incorrect usage of Node.js internals.\n' +
6 changes: 6 additions & 0 deletions src/inspector/worker_inspector.cc
Original file line number Diff line number Diff line change
@@ -72,6 +72,12 @@ void ParentInspectorHandle::WorkerStarted(
parent_thread_->Post(std::move(request));
}

std::unique_ptr<inspector::InspectorSession> ParentInspectorHandle::Connect(
std::unique_ptr<inspector::InspectorSessionDelegate> delegate,
bool prevent_shutdown) {
return parent_thread_->Connect(std::move(delegate), prevent_shutdown);
}

void WorkerManager::WorkerFinished(int session_id) {
children_.erase(session_id);
}
5 changes: 5 additions & 0 deletions src/inspector/worker_inspector.h
Original file line number Diff line number Diff line change
@@ -12,6 +12,8 @@

namespace node {
namespace inspector {
class InspectorSession;
class InspectorSessionDelegate;
class MainThreadHandle;
class WorkerManager;

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

private:
int id_;
11 changes: 11 additions & 0 deletions src/inspector_agent.cc
Original file line number Diff line number Diff line change
@@ -828,6 +828,17 @@ std::unique_ptr<InspectorSession> Agent::Connect(
new SameThreadInspectorSession(session_id, client_));
}

std::unique_ptr<InspectorSession> Agent::ConnectToMainThread(
std::unique_ptr<InspectorSessionDelegate> delegate,
bool prevent_shutdown) {
CHECK_NOT_NULL(parent_handle_);
CHECK_NOT_NULL(client_);
auto thread_safe_delegate =
client_->getThreadHandle()->MakeDelegateThreadSafe(std::move(delegate));
return parent_handle_->Connect(std::move(thread_safe_delegate),
prevent_shutdown);
}

void Agent::WaitForDisconnect() {
CHECK_NOT_NULL(client_);
bool is_worker = parent_handle_ != nullptr;
9 changes: 8 additions & 1 deletion src/inspector_agent.h
Original file line number Diff line number Diff line change
@@ -86,12 +86,19 @@ class Agent {
std::unique_ptr<ParentInspectorHandle> GetParentHandle(
int thread_id, const std::string& url);

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

// Called from the worker to create inspector sessions that is connected
// to the main thread.
// The inspector responds by using the delegate to send messages back.
std::unique_ptr<InspectorSession> ConnectToMainThread(
std::unique_ptr<InspectorSessionDelegate> delegate,
bool prevent_shutdown);

void PauseOnNextJavascriptStatement(const std::string& reason);

std::string GetWsUrl() const;
56 changes: 42 additions & 14 deletions src/inspector_js_api.cc
Original file line number Diff line number Diff line change
@@ -37,6 +37,29 @@ std::unique_ptr<StringBuffer> ToProtocolString(Isolate* isolate,
return StringBuffer::create(StringView(*buffer, buffer.length()));
}

struct LocalConnection {
static std::unique_ptr<InspectorSession> Connect(
Agent* inspector, std::unique_ptr<InspectorSessionDelegate> delegate) {
return inspector->Connect(std::move(delegate), false);
}

static Local<String> GetClassName(Environment* env) {
return FIXED_ONE_BYTE_STRING(env->isolate(), "Connection");
}
};

struct MainThreadConnection {
static std::unique_ptr<InspectorSession> Connect(
Agent* inspector, std::unique_ptr<InspectorSessionDelegate> delegate) {
return inspector->ConnectToMainThread(std::move(delegate), true);
}

static Local<String> GetClassName(Environment* env) {
return FIXED_ONE_BYTE_STRING(env->isolate(), "MainThreadConnection");
}
};

template <typename ConnectionType>
class JSBindingsConnection : public AsyncWrap {
public:
class JSBindingsSessionDelegate : public InspectorSessionDelegate {
@@ -70,14 +93,29 @@ class JSBindingsConnection : public AsyncWrap {
: AsyncWrap(env, wrap, PROVIDER_INSPECTORJSBINDING),
callback_(env->isolate(), callback) {
Agent* inspector = env->inspector_agent();
session_ = inspector->Connect(std::make_unique<JSBindingsSessionDelegate>(
env, this), false);
session_ = ConnectionType::Connect(
inspector, std::make_unique<JSBindingsSessionDelegate>(env, this));
}

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

static void Bind(Environment* env, Local<Object> target) {
Local<String> class_name = ConnectionType::GetClassName(env);
Local<FunctionTemplate> tmpl =
env->NewFunctionTemplate(JSBindingsConnection::New);
tmpl->InstanceTemplate()->SetInternalFieldCount(1);
tmpl->SetClassName(class_name);
tmpl->Inherit(AsyncWrap::GetConstructorTemplate(env));
env->SetProtoMethod(tmpl, "dispatch", JSBindingsConnection::Dispatch);
env->SetProtoMethod(tmpl, "disconnect", JSBindingsConnection::Disconnect);
target->Set(env->context(),
class_name,
tmpl->GetFunction(env->context()).ToLocalChecked())
.ToChecked();
}

static void New(const FunctionCallbackInfo<Value>& info) {
Environment* env = Environment::GetCurrent(info);
CHECK(info[0]->IsFunction());
@@ -300,18 +338,8 @@ void Initialize(Local<Object> target, Local<Value> unused,
env->SetMethod(target, "registerAsyncHook", RegisterAsyncHookWrapper);
env->SetMethodNoSideEffect(target, "isEnabled", IsEnabled);

auto conn_str = FIXED_ONE_BYTE_STRING(env->isolate(), "Connection");
Local<FunctionTemplate> tmpl =
env->NewFunctionTemplate(JSBindingsConnection::New);
tmpl->InstanceTemplate()->SetInternalFieldCount(1);
tmpl->SetClassName(conn_str);
tmpl->Inherit(AsyncWrap::GetConstructorTemplate(env));
env->SetProtoMethod(tmpl, "dispatch", JSBindingsConnection::Dispatch);
env->SetProtoMethod(tmpl, "disconnect", JSBindingsConnection::Disconnect);
target
->Set(env->context(), conn_str,
tmpl->GetFunction(env->context()).ToLocalChecked())
.ToChecked();
JSBindingsConnection<LocalConnection>::Bind(env, target);
JSBindingsConnection<MainThreadConnection>::Bind(env, target);
}

} // namespace
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// Flags: --expose-internals
'use strict';
const common = require('../common');
common.skipIfInspectorDisabled();
@@ -86,7 +85,7 @@ function testSampleDebugSession() {
}, TypeError);
session.post('Debugger.enable', () => cbAsSecondArgCalled = true);
session.post('Debugger.setBreakpointByUrl', {
'lineNumber': 14,
'lineNumber': 13,
'url': pathToFileURL(path.resolve(__dirname, __filename)).toString(),
'columnNumber': 0,
'condition': ''
Loading

0 comments on commit 3d841fe

Please sign in to comment.