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.

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

WS connection should not crash with JS bindings session #17085

Merged
merged 1 commit into from
Dec 12, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 0 additions & 4 deletions src/inspector_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -548,10 +548,6 @@ void Agent::Connect(InspectorSessionDelegate* delegate) {
client_->connectFrontend(delegate);
}

bool Agent::IsConnected() {
return io_ && io_->IsConnected();
}

void Agent::WaitForDisconnect() {
CHECK_NE(client_, nullptr);
client_->contextDestroyed(parent_env_->context());
Expand Down
1 change: 0 additions & 1 deletion src/inspector_agent.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ class Agent {
bool IsStarted() { return !!client_; }

// IO thread started, and client connected
bool IsConnected();
bool IsWaitingForConnect();

void WaitForDisconnect();
Expand Down
73 changes: 47 additions & 26 deletions src/inspector_io.cc
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ class InspectorIoDelegate: public node::inspector::SocketServerDelegate {
const std::string& script_name, bool wait);
// Calls PostIncomingMessage() with appropriate InspectorAction:
// kStartSession
bool StartSession(int session_id, const std::string& target_id) override;
void StartSession(int session_id, const std::string& target_id) override;
// kSendMessage
void MessageReceived(int session_id, const std::string& message) override;
// kEndSession
Expand All @@ -145,19 +145,22 @@ class InspectorIoDelegate: public node::inspector::SocketServerDelegate {
std::vector<std::string> GetTargetIds() override;
std::string GetTargetTitle(const std::string& id) override;
std::string GetTargetUrl(const std::string& id) override;
bool IsConnected() { return connected_; }
void ServerDone() override {
io_->ServerDone();
}

void AssignTransport(InspectorSocketServer* server) {
server_ = server;
}

private:
InspectorIo* io_;
bool connected_;
int session_id_;
const std::string script_name_;
const std::string script_path_;
const std::string target_id_;
bool waiting_;
InspectorSocketServer* server_;
};

void InterruptCallback(v8::Isolate*, void* agent) {
Expand Down Expand Up @@ -226,10 +229,6 @@ void InspectorIo::Stop() {
DispatchMessages();
}

bool InspectorIo::IsConnected() {
return delegate_ != nullptr && delegate_->IsConnected();
}

bool InspectorIo::IsStarted() {
return platform_ != nullptr;
}
Expand Down Expand Up @@ -264,6 +263,7 @@ void InspectorIo::IoThreadAsyncCb(uv_async_t* async) {
MessageQueue<TransportAction> outgoing_message_queue;
io->SwapBehindLock(&io->outgoing_message_queue_, &outgoing_message_queue);
for (const auto& outgoing : outgoing_message_queue) {
int session_id = std::get<1>(outgoing);
switch (std::get<0>(outgoing)) {
case TransportAction::kKill:
transport->TerminateConnections();
Expand All @@ -272,8 +272,14 @@ void InspectorIo::IoThreadAsyncCb(uv_async_t* async) {
transport->Stop(nullptr);
break;
case TransportAction::kSendMessage:
std::string message = StringViewToUtf8(std::get<2>(outgoing)->string());
transport->Send(std::get<1>(outgoing), message);
transport->Send(session_id,
StringViewToUtf8(std::get<2>(outgoing)->string()));
break;
case TransportAction::kAcceptSession:
transport->AcceptSession(session_id);
break;
case TransportAction::kDeclineSession:
transport->DeclineSession(session_id);
break;
}
}
Expand All @@ -293,6 +299,7 @@ void InspectorIo::ThreadMain() {
wait_for_connect_);
delegate_ = &delegate;
Transport server(&delegate, &loop, options_.host_name(), options_.port());
delegate.AssignTransport(&server);
TransportAndIo<Transport> queue_transport(&server, this);
thread_req_.data = &queue_transport;
if (!server.Start()) {
Expand All @@ -308,6 +315,7 @@ void InspectorIo::ThreadMain() {
uv_run(&loop, UV_RUN_DEFAULT);
thread_req_.data = nullptr;
CHECK_EQ(uv_loop_close(&loop), 0);
delegate.AssignTransport(nullptr);
delegate_ = nullptr;
}

Expand Down Expand Up @@ -358,6 +366,21 @@ void InspectorIo::NotifyMessageReceived() {
incoming_message_cond_.Broadcast(scoped_lock);
}

TransportAction InspectorIo::Attach(int session_id) {
Agent* agent = parent_env_->inspector_agent();
if (agent->delegate() != nullptr)
return TransportAction::kDeclineSession;

CHECK_EQ(session_delegate_, nullptr);
session_id_ = session_id;
state_ = State::kConnected;
fprintf(stderr, "Debugger attached.\n");
session_delegate_ = std::unique_ptr<InspectorSessionDelegate>(
new IoSessionDelegate(this));
agent->Connect(session_delegate_.get());
return TransportAction::kAcceptSession;
}

void InspectorIo::DispatchMessages() {
// This function can be reentered if there was an incoming message while
// V8 was processing another inspector request (e.g. if the user is
Expand All @@ -375,16 +398,14 @@ void InspectorIo::DispatchMessages() {
MessageQueue<InspectorAction>::value_type task;
std::swap(dispatching_message_queue_.front(), task);
dispatching_message_queue_.pop_front();
int id = std::get<1>(task);
StringView message = std::get<2>(task)->string();
switch (std::get<0>(task)) {
case InspectorAction::kStartSession:
CHECK_EQ(session_delegate_, nullptr);
session_id_ = std::get<1>(task);
state_ = State::kConnected;
fprintf(stderr, "Debugger attached.\n");
session_delegate_ = std::unique_ptr<InspectorSessionDelegate>(
new IoSessionDelegate(this));
parent_env_->inspector_agent()->Connect(session_delegate_.get());
Write(Attach(id), id, StringView());
break;
case InspectorAction::kStartSessionUnconditionally:
Attach(id);
break;
case InspectorAction::kEndSession:
CHECK_NE(session_delegate_, nullptr);
Expand Down Expand Up @@ -428,22 +449,23 @@ InspectorIoDelegate::InspectorIoDelegate(InspectorIo* io,
const std::string& script_name,
bool wait)
: io_(io),
connected_(false),
session_id_(0),
script_name_(script_name),
script_path_(script_path),
target_id_(GenerateID()),
waiting_(wait) { }
waiting_(wait),
server_(nullptr) { }


bool InspectorIoDelegate::StartSession(int session_id,
void InspectorIoDelegate::StartSession(int session_id,
const std::string& target_id) {
if (connected_)
return false;
connected_ = true;
session_id_++;
io_->PostIncomingMessage(InspectorAction::kStartSession, session_id, "");
return true;
session_id_ = session_id;
InspectorAction action = InspectorAction::kStartSession;
if (waiting_) {
action = InspectorAction::kStartSessionUnconditionally;
server_->AcceptSession(session_id);
}
io_->PostIncomingMessage(action, session_id, "");
}

void InspectorIoDelegate::MessageReceived(int session_id,
Expand All @@ -464,7 +486,6 @@ void InspectorIoDelegate::MessageReceived(int session_id,
}

void InspectorIoDelegate::EndSession(int session_id) {
connected_ = false;
io_->PostIncomingMessage(InspectorAction::kEndSession, session_id, "");
}

Expand Down
8 changes: 6 additions & 2 deletions src/inspector_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class InspectorIoDelegate;

enum class InspectorAction {
kStartSession,
kStartSessionUnconditionally, // First attach with --inspect-brk
kEndSession,
kSendMessage
};
Expand All @@ -44,7 +45,9 @@ enum class InspectorAction {
enum class TransportAction {
kKill,
kSendMessage,
kStop
kStop,
kAcceptSession,
kDeclineSession
};

class InspectorIo {
Expand All @@ -61,7 +64,6 @@ class InspectorIo {
void Stop();

bool IsStarted();
bool IsConnected();

void WaitForDisconnect();
// Called from thread to queue an incoming message and trigger
Expand Down Expand Up @@ -124,6 +126,8 @@ class InspectorIo {
void WaitForFrontendMessageWhilePaused();
// Broadcast incoming_message_cond_
void NotifyMessageReceived();
// Attach session to an inspector. Either kAcceptSession or kDeclineSession
TransportAction Attach(int session_id);

const DebugOptions options_;

Expand Down
Loading