Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

inspector: allow concurrent inspector sessions #20137

Closed
wants to merge 1 commit into from
Closed

inspector: allow concurrent inspector sessions #20137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 70 additions & 66 deletions src/inspector_agent.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,9 @@ const int CONTEXT_GROUP_ID = 1;

class ChannelImpl final : public v8_inspector::V8Inspector::Channel {
public:
explicit ChannelImpl(V8Inspector* inspector,
InspectorSessionDelegate* delegate)
: delegate_(delegate) {
explicit ChannelImpl(const std::unique_ptr<V8Inspector>& inspector,
std::unique_ptr<InspectorSessionDelegate> delegate)
: delegate_(std::move(delegate)) {
session_ = inspector->connect(1, this, StringView());
}

Expand All @@ -201,19 +201,11 @@ class ChannelImpl final : public v8_inspector::V8Inspector::Channel {
session_->dispatchProtocolMessage(message);
}

bool waitForFrontendMessage() {
return delegate_->WaitForFrontendMessageWhilePaused();
}

void schedulePauseOnNextStatement(const std::string& reason) {
std::unique_ptr<StringBuffer> buffer = Utf8ToStringView(reason);
session_->schedulePauseOnNextStatement(buffer->string(), buffer->string());
}

InspectorSessionDelegate* delegate() {
return delegate_;
}

private:
void sendResponse(
int callId,
Expand All @@ -232,7 +224,7 @@ class ChannelImpl final : public v8_inspector::V8Inspector::Channel {
delegate_->SendMessageToFrontend(message);
}

InspectorSessionDelegate* const delegate_;
std::unique_ptr<InspectorSessionDelegate> delegate_;
std::unique_ptr<v8_inspector::V8InspectorSession> session_;
};

Expand Down Expand Up @@ -300,8 +292,7 @@ class InspectorTimerHandle {
class NodeInspectorClient : public V8InspectorClient {
public:
NodeInspectorClient(node::Environment* env, node::NodePlatform* platform)
: env_(env), platform_(platform), terminated_(false),
running_nested_loop_(false) {
: env_(env), platform_(platform) {
client_ = V8Inspector::create(env->isolate(), this);
// TODO(bnoordhuis) Make name configurable from src/node.cc.
ContextInfo info(GetHumanReadableProcessName());
Expand All @@ -310,18 +301,28 @@ class NodeInspectorClient : public V8InspectorClient {
}

void runMessageLoopOnPause(int context_group_id) override {
CHECK_NE(channel_, nullptr);
runMessageLoop(false);
}

void runMessageLoop(bool ignore_terminated) {
if (running_nested_loop_)
return;
terminated_ = false;
running_nested_loop_ = true;
while (!terminated_ && channel_->waitForFrontendMessage()) {
while ((ignore_terminated || !terminated_) && waitForFrontendEvent()) {
while (platform_->FlushForegroundTasks(env_->isolate())) {}
}
terminated_ = false;
running_nested_loop_ = false;
}

bool waitForFrontendEvent() {
InspectorIo* io = env_->inspector_agent()->io();
if (io == nullptr)
return false;
return io->WaitForFrontendEvent();
}

double currentTimeMS() override {
return uv_hrtime() * 1.0 / NANOS_PER_MSEC;
}
Expand Down Expand Up @@ -363,20 +364,22 @@ class NodeInspectorClient : public V8InspectorClient {
terminated_ = true;
}

void connectFrontend(InspectorSessionDelegate* delegate) {
CHECK_EQ(channel_, nullptr);
channel_ = std::unique_ptr<ChannelImpl>(
new ChannelImpl(client_.get(), delegate));
int connectFrontend(std::unique_ptr<InspectorSessionDelegate> delegate) {
events_dispatched_ = true;
int session_id = next_session_id_++;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible for this to have a race condition (e.g. WebSocket server and a JS inspector session launching at the same time)?

Copy link
Contributor Author

@eugeneo eugeneo Apr 19, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All API in this file are called only from the main thread. E.g. incoming WS session will "interrupt" V8 (or will use uv_async_t) before calling this function.

channels_[session_id] =
std::make_unique<ChannelImpl>(client_, std::move(delegate));
return session_id;
}

void disconnectFrontend() {
quitMessageLoopOnPause();
channel_.reset();
void disconnectFrontend(int session_id) {
events_dispatched_ = true;
channels_.erase(session_id);
}

void dispatchMessageFromFrontend(const StringView& message) {
CHECK_NE(channel_, nullptr);
channel_->dispatchProtocolMessage(message);
void dispatchMessageFromFrontend(int session_id, const StringView& message) {
events_dispatched_ = true;
channels_[session_id]->dispatchProtocolMessage(message);
}

Local<Context> ensureDefaultContextInGroup(int contextGroupId) override {
Expand Down Expand Up @@ -426,10 +429,6 @@ class NodeInspectorClient : public V8InspectorClient {
script_id);
}

ChannelImpl* channel() {
return channel_.get();
}

void startRepeatingTimer(double interval_s,
TimerCallback callback,
void* data) override {
Expand Down Expand Up @@ -464,20 +463,31 @@ class NodeInspectorClient : public V8InspectorClient {
client_->allAsyncTasksCanceled();
}

void schedulePauseOnNextStatement(const std::string& reason) {
for (const auto& id_channel : channels_) {
id_channel.second->schedulePauseOnNextStatement(reason);
}
}

bool hasConnectedSessions() {
return !channels_.empty();
}

private:
node::Environment* env_;
node::NodePlatform* platform_;
bool terminated_;
bool running_nested_loop_;
bool terminated_ = false;
bool running_nested_loop_ = false;
std::unique_ptr<V8Inspector> client_;
std::unique_ptr<ChannelImpl> channel_;
std::unordered_map<int, std::unique_ptr<ChannelImpl>> channels_;
std::unordered_map<void*, InspectorTimerHandle> timers_;
int next_session_id_ = 1;
bool events_dispatched_ = false;
};

Agent::Agent(Environment* env) : parent_env_(env),
client_(nullptr),
platform_(nullptr),
enabled_(false),
pending_enable_async_hook_(false),
pending_disable_async_hook_(false) {}

Expand All @@ -491,7 +501,7 @@ bool Agent::Start(node::NodePlatform* platform, const char* path,
path_ = path == nullptr ? "" : path;
debug_options_ = options;
client_ =
std::unique_ptr<NodeInspectorClient>(
std::shared_ptr<NodeInspectorClient>(
new NodeInspectorClient(parent_env_, platform));
platform_ = platform;
CHECK_EQ(0, uv_async_init(uv_default_loop(),
Expand All @@ -515,7 +525,6 @@ bool Agent::StartIoThread(bool wait_for_connect) {

CHECK_NE(client_, nullptr);

enabled_ = true;
io_ = std::unique_ptr<InspectorIo>(
new InspectorIo(parent_env_, platform_, path_, debug_options_,
wait_for_connect));
Expand Down Expand Up @@ -554,20 +563,25 @@ void Agent::Stop() {
if (io_ != nullptr) {
io_->Stop();
io_.reset();
enabled_ = false;
}
}

void Agent::Connect(InspectorSessionDelegate* delegate) {
enabled_ = true;
client_->connectFrontend(delegate);
std::unique_ptr<InspectorSession> Agent::Connect(
std::unique_ptr<InspectorSessionDelegate> delegate) {
int session_id = client_->connectFrontend(std::move(delegate));
return std::make_unique<InspectorSession>(session_id, client_);
}

void Agent::WaitForDisconnect() {
CHECK_NE(client_, nullptr);
client_->contextDestroyed(parent_env_->context());
if (io_ != nullptr) {
io_->WaitForDisconnect();
// There is a bug in V8 Inspector (https://crbug.com/834056) that
// calls V8InspectorClient::quitMessageLoopOnPause when a session
// disconnects. We are using this flag to ignore those calls so the message
// loop is spinning as long as there's a reason to expect inspector messages
client_->runMessageLoop(true);
}
}

Expand All @@ -578,33 +592,8 @@ void Agent::FatalException(Local<Value> error, Local<v8::Message> message) {
WaitForDisconnect();
}

void Agent::Dispatch(const StringView& message) {
CHECK_NE(client_, nullptr);
client_->dispatchMessageFromFrontend(message);
}

void Agent::Disconnect() {
CHECK_NE(client_, nullptr);
client_->disconnectFrontend();
}

void Agent::RunMessageLoop() {
CHECK_NE(client_, nullptr);
client_->runMessageLoopOnPause(CONTEXT_GROUP_ID);
}

InspectorSessionDelegate* Agent::delegate() {
CHECK_NE(client_, nullptr);
ChannelImpl* channel = client_->channel();
if (channel == nullptr)
return nullptr;
return channel->delegate();
}

void Agent::PauseOnNextJavascriptStatement(const std::string& reason) {
ChannelImpl* channel = client_->channel();
if (channel != nullptr)
channel->schedulePauseOnNextStatement(reason);
client_->schedulePauseOnNextStatement(reason);
}

void Agent::RegisterAsyncHook(Isolate* isolate,
Expand Down Expand Up @@ -699,5 +688,20 @@ bool Agent::IsWaitingForConnect() {
return debug_options_.wait_for_connect();
}

bool Agent::HasConnectedSessions() {
return client_->hasConnectedSessions();
}

InspectorSession::InspectorSession(int session_id,
std::shared_ptr<NodeInspectorClient> client)
: session_id_(session_id), client_(client) {}

InspectorSession::~InspectorSession() {
client_->disconnectFrontend(session_id_);
}

void InspectorSession::Dispatch(const StringView& message) {
client_->dispatchMessageFromFrontend(session_id_, message);
}
} // namespace inspector
} // namespace node
40 changes: 22 additions & 18 deletions src/inspector_agent.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,26 @@ class Environment;
struct ContextInfo;

namespace inspector {
class InspectorIo;
class NodeInspectorClient;

class InspectorSession {
public:
InspectorSession(int session_id, std::shared_ptr<NodeInspectorClient> client);
~InspectorSession();
void Dispatch(const v8_inspector::StringView& message);
private:
int session_id_;
std::shared_ptr<NodeInspectorClient> client_;
};

class InspectorSessionDelegate {
public:
virtual ~InspectorSessionDelegate() = default;
virtual bool WaitForFrontendMessageWhilePaused() = 0;
virtual void SendMessageToFrontend(const v8_inspector::StringView& message)
= 0;
};

class InspectorIo;
class NodeInspectorClient;

class Agent {
public:
explicit Agent(node::Environment* env);
Expand Down Expand Up @@ -66,19 +74,19 @@ class Agent {
void RegisterAsyncHook(v8::Isolate* isolate,
v8::Local<v8::Function> enable_function,
v8::Local<v8::Function> disable_function);
void EnableAsyncHook();
void DisableAsyncHook();

// These methods are called by the WS protocol and JS binding to create
// inspector sessions. The inspector responds by using the delegate to send
// messages back.
void Connect(InspectorSessionDelegate* delegate);
void Disconnect();
void Dispatch(const v8_inspector::StringView& message);
InspectorSessionDelegate* delegate();
// Called by the WS protocol and JS binding to create inspector sessions.
// The inspector responds by using the delegate to send messages back.
std::unique_ptr<InspectorSession> Connect(
std::unique_ptr<InspectorSessionDelegate> delegate);

void RunMessageLoop();
bool enabled() { return enabled_; }
void PauseOnNextJavascriptStatement(const std::string& reason);

// Returns true as long as there is at least one connected session.
bool HasConnectedSessions();

InspectorIo* io() {
return io_.get();
}
Expand All @@ -92,18 +100,14 @@ class Agent {
DebugOptions& options() { return debug_options_; }
void ContextCreated(v8::Local<v8::Context> context, const ContextInfo& info);

void EnableAsyncHook();
void DisableAsyncHook();

private:
void ToggleAsyncHook(v8::Isolate* isolate,
const Persistent<v8::Function>& fn);

node::Environment* parent_env_;
std::unique_ptr<NodeInspectorClient> client_;
std::shared_ptr<NodeInspectorClient> client_;
std::unique_ptr<InspectorIo> io_;
v8::Platform* platform_;
bool enabled_;
std::string path_;
DebugOptions debug_options_;

Expand Down
Loading