From 2d08bbadd6eb5aab9b66d05b68f9f8ce57680e76 Mon Sep 17 00:00:00 2001 From: Eugene Ostroukhov Date: Mon, 21 Nov 2016 09:38:19 -0800 Subject: [PATCH] inspector: stop relying on magic strings Inspector uses magical strings to communicate some events between main thread and transport thread. This change replaces those strings with enums that are more mainatainable (and remove unnecessary encodings/decodings) PR-URL: https://github.com/nodejs/node/pull/10159 Reviewed-By: Ben Noordhuis --- src/inspector_agent.cc | 106 ++++++++++++++++++++++++----------------- 1 file changed, 61 insertions(+), 45 deletions(-) diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index dba4e0f9826004..2173af146855fd 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -16,6 +16,7 @@ #include #include +#include #include #include @@ -30,9 +31,6 @@ namespace { using v8_inspector::StringBuffer; using v8_inspector::StringView; -const char TAG_CONNECT[] = "#connect"; -const char TAG_DISCONNECT[] = "#disconnect"; - static const uint8_t PROTOCOL_JSON[] = { #include "v8_inspector_protocol_json.h" // NOLINT(build/include_order) }; @@ -105,6 +103,14 @@ std::unique_ptr Utf8ToStringView(const std::string& message) { class V8NodeInspector; +enum class InspectorAction { + kStartSession, kEndSession, kSendMessage +}; + +enum class TransportAction { + kSendMessage, kStop +}; + class InspectorAgentDelegate: public node::inspector::SocketServerDelegate { public: InspectorAgentDelegate(AgentImpl* agent, const std::string& script_path, @@ -143,14 +149,16 @@ class AgentImpl { void FatalException(v8::Local error, v8::Local message); - void PostIncomingMessage(int session_id, const std::string& message); + void PostIncomingMessage(InspectorAction action, int session_id, + const std::string& message); void ResumeStartup() { uv_sem_post(&start_sem_); } private: + template using MessageQueue = - std::vector>>; + std::vector>>; enum class State { kNew, kAccepting, kConnected, kDone, kError }; static void ThreadCbIO(void* agent); @@ -161,10 +169,13 @@ class AgentImpl { void WorkerRunIO(); void SetConnected(bool connected); void DispatchMessages(); - void Write(int session_id, const StringView& message); - bool AppendMessage(MessageQueue* vector, int session_id, - std::unique_ptr buffer); - void SwapBehindLock(MessageQueue* vector1, MessageQueue* vector2); + void Write(TransportAction action, int session_id, const StringView& message); + template + bool AppendMessage(MessageQueue* vector, ActionType action, + int session_id, std::unique_ptr buffer); + template + void SwapBehindLock(MessageQueue* vector1, + MessageQueue* vector2); void WaitForFrontendMessage(); void NotifyMessageReceived(); State ToState(State state); @@ -185,8 +196,8 @@ class AgentImpl { uv_async_t io_thread_req_; V8NodeInspector* inspector_; v8::Platform* platform_; - MessageQueue incoming_message_queue_; - MessageQueue outgoing_message_queue_; + MessageQueue incoming_message_queue_; + MessageQueue outgoing_message_queue_; bool dispatching_messages_; int session_id_; InspectorSocketServer* server_; @@ -236,7 +247,7 @@ class ChannelImpl final : public v8_inspector::V8Inspector::Channel { void flushProtocolNotifications() override { } void sendMessageToFrontend(const StringView& message) { - agent_->Write(agent_->session_id_, message); + agent_->Write(TransportAction::kSendMessage, agent_->session_id_, message); } AgentImpl* const agent_; @@ -444,9 +455,7 @@ bool AgentImpl::IsStarted() { void AgentImpl::WaitForDisconnect() { if (state_ == State::kConnected) { shutting_down_ = true; - // Gives a signal to stop accepting new connections - // TODO(eugeneo): Introduce an API with explicit request names. - Write(0, StringView()); + Write(TransportAction::kStop, 0, StringView()); fprintf(stderr, "Waiting for the debugger to disconnect...\n"); fflush(stderr); inspector_->runMessageLoopOnPause(0); @@ -521,15 +530,17 @@ void AgentImpl::ThreadCbIO(void* agent) { // static void AgentImpl::WriteCbIO(uv_async_t* async) { AgentImpl* agent = static_cast(async->data); - MessageQueue outgoing_messages; + MessageQueue outgoing_messages; agent->SwapBehindLock(&agent->outgoing_message_queue_, &outgoing_messages); - for (const MessageQueue::value_type& outgoing : outgoing_messages) { - StringView view = outgoing.second->string(); - if (view.length() == 0) { + for (const auto& outgoing : outgoing_messages) { + switch (std::get<0>(outgoing)) { + case TransportAction::kStop: agent->server_->Stop(nullptr); - } else { - agent->server_->Send(outgoing.first, - StringViewToUtf8(outgoing.second->string())); + break; + case TransportAction::kSendMessage: + std::string message = StringViewToUtf8(std::get<2>(outgoing)->string()); + agent->server_->Send(std::get<1>(outgoing), message); + break; } } } @@ -573,22 +584,26 @@ void AgentImpl::WorkerRunIO() { server_ = nullptr; } -bool AgentImpl::AppendMessage(MessageQueue* queue, int session_id, +template +bool AgentImpl::AppendMessage(MessageQueue* queue, + ActionType action, int session_id, std::unique_ptr buffer) { Mutex::ScopedLock scoped_lock(state_lock_); bool trigger_pumping = queue->empty(); - queue->push_back(std::make_pair(session_id, std::move(buffer))); + queue->push_back(std::make_tuple(action, session_id, std::move(buffer))); return trigger_pumping; } -void AgentImpl::SwapBehindLock(MessageQueue* vector1, MessageQueue* vector2) { +template +void AgentImpl::SwapBehindLock(MessageQueue* vector1, + MessageQueue* vector2) { Mutex::ScopedLock scoped_lock(state_lock_); vector1->swap(*vector2); } -void AgentImpl::PostIncomingMessage(int session_id, +void AgentImpl::PostIncomingMessage(InspectorAction action, int session_id, const std::string& message) { - if (AppendMessage(&incoming_message_queue_, session_id, + if (AppendMessage(&incoming_message_queue_, action, session_id, Utf8ToStringView(message))) { v8::Isolate* isolate = parent_env_->isolate(); platform_->CallOnForegroundThread(isolate, @@ -617,25 +632,21 @@ void AgentImpl::DispatchMessages() { if (dispatching_messages_) return; dispatching_messages_ = true; - MessageQueue tasks; + MessageQueue tasks; do { tasks.clear(); SwapBehindLock(&incoming_message_queue_, &tasks); - for (const MessageQueue::value_type& pair : tasks) { - StringView message = pair.second->string(); - std::string tag; - if (message.length() == sizeof(TAG_CONNECT) - 1 || - message.length() == sizeof(TAG_DISCONNECT) - 1) { - tag = StringViewToUtf8(message); - } - - if (tag == TAG_CONNECT) { + for (const auto& task : tasks) { + StringView message = std::get<2>(task)->string(); + switch (std::get<0>(task)) { + case InspectorAction::kStartSession: CHECK_EQ(State::kAccepting, state_); - session_id_ = pair.first; + session_id_ = std::get<1>(task); state_ = State::kConnected; fprintf(stderr, "Debugger attached.\n"); inspector_->connectFrontend(); - } else if (tag == TAG_DISCONNECT) { + break; + case InspectorAction::kEndSession: CHECK_EQ(State::kConnected, state_); if (shutting_down_) { state_ = State::kDone; @@ -644,16 +655,19 @@ void AgentImpl::DispatchMessages() { } inspector_->quitMessageLoopOnPause(); inspector_->disconnectFrontend(); - } else { + break; + case InspectorAction::kSendMessage: inspector_->dispatchMessageFromFrontend(message); + break; } } } while (!tasks.empty()); dispatching_messages_ = false; } -void AgentImpl::Write(int session_id, const StringView& inspector_message) { - AppendMessage(&outgoing_message_queue_, session_id, +void AgentImpl::Write(TransportAction action, int session_id, + const StringView& inspector_message) { + AppendMessage(&outgoing_message_queue_, action, session_id, StringBuffer::create(inspector_message)); int err = uv_async_send(&io_thread_req_); CHECK_EQ(0, err); @@ -710,7 +724,8 @@ bool InspectorAgentDelegate::StartSession(int session_id, if (connected_) return false; connected_ = true; - agent_->PostIncomingMessage(session_id, TAG_CONNECT); + session_id_++; + agent_->PostIncomingMessage(InspectorAction::kStartSession, session_id, ""); return true; } @@ -727,12 +742,13 @@ void InspectorAgentDelegate::MessageReceived(int session_id, agent_->ResumeStartup(); } } - agent_->PostIncomingMessage(session_id, message); + agent_->PostIncomingMessage(InspectorAction::kSendMessage, session_id, + message); } void InspectorAgentDelegate::EndSession(int session_id) { connected_ = false; - agent_->PostIncomingMessage(session_id, TAG_DISCONNECT); + agent_->PostIncomingMessage(InspectorAction::kEndSession, session_id, ""); } std::vector InspectorAgentDelegate::GetTargetIds() {