From a584ca36783e715a2f808fec134310dfddaddd62 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Thu, 4 Sep 2014 21:06:31 +0800 Subject: [PATCH 1/8] Start a new debugger thread to listen for commands. --- atom/browser/node_debugger.cc | 52 +++++++++++++++++------------------ atom/browser/node_debugger.h | 19 +++++++++---- 2 files changed, 40 insertions(+), 31 deletions(-) diff --git a/atom/browser/node_debugger.cc b/atom/browser/node_debugger.cc index 0e7d58b1dd49..007180e8bd6c 100644 --- a/atom/browser/node_debugger.cc +++ b/atom/browser/node_debugger.cc @@ -7,24 +7,18 @@ #include #include "atom/common/atom_version.h" +#include "base/bind.h" #include "base/command_line.h" #include "base/strings/string_number_conversions.h" -#include "v8/include/v8.h" +#include "net/socket/stream_socket.h" +#include "net/socket/tcp_server_socket.h" #include "v8/include/v8-debug.h" -#include "atom/common/node_includes.h" - namespace atom { -// static -uv_async_t NodeDebugger::dispatch_debug_messages_async_; - -NodeDebugger::NodeDebugger() { - uv_async_init(uv_default_loop(), - &dispatch_debug_messages_async_, - DispatchDebugMessagesInMainThread); - uv_unref(reinterpret_cast(&dispatch_debug_messages_async_)); - +NodeDebugger::NodeDebugger() + : thread_("NodeDebugger"), + weak_factory_(this) { bool use_debug_agent = false; int port = 5858; bool wait_for_connection = false; @@ -44,26 +38,32 @@ NodeDebugger::NodeDebugger() { if (use_debug_agent) { if (!port_str.empty()) base::StringToInt(port_str, &port); -#if 0 - v8::Debug::EnableAgent("atom-shell " ATOM_VERSION, port, - wait_for_connection); - v8::Debug::SetDebugMessageDispatchHandler(DispatchDebugMessagesInMsgThread, - false); -#endif + + if (!thread_.Start()) { + LOG(ERROR) << "Unable to start debugger thread"; + return; + } + + net::IPAddressNumber ip_number; + if (!net::ParseIPLiteralToNumber("127.0.0.1", &ip_number)) { + LOG(ERROR) << "Unable to convert ip address"; + return; + } + + server_socket_.reset(new net::TCPServerSocket(NULL, net::NetLog::Source())); + server_socket_->Listen(net::IPEndPoint(ip_number, port), 1); + + thread_.message_loop()->PostTask( + FROM_HERE, + base::Bind(&NodeDebugger::DoAcceptLoop, weak_factory_.GetWeakPtr())); } } NodeDebugger::~NodeDebugger() { + thread_.Stop(); } -// static -void NodeDebugger::DispatchDebugMessagesInMainThread(uv_async_t* handle) { - v8::Debug::ProcessDebugMessages(); -} - -// static -void NodeDebugger::DispatchDebugMessagesInMsgThread() { - uv_async_send(&dispatch_debug_messages_async_); +void NodeDebugger::DoAcceptLoop() { } } // namespace atom diff --git a/atom/browser/node_debugger.h b/atom/browser/node_debugger.h index 4dfdced8ee9d..a0eb3a6b4632 100644 --- a/atom/browser/node_debugger.h +++ b/atom/browser/node_debugger.h @@ -5,8 +5,14 @@ #ifndef ATOM_BROWSER_NODE_DEBUGGER_H_ #define ATOM_BROWSER_NODE_DEBUGGER_H_ -#include "base/basictypes.h" -#include "vendor/node/deps/uv/include/uv.h" +#include "base/memory/scoped_ptr.h" +#include "base/memory/weak_ptr.h" +#include "base/threading/thread.h" + +namespace net { +class StreamSocket; +class TCPServerSocket; +} namespace atom { @@ -17,10 +23,13 @@ class NodeDebugger { virtual ~NodeDebugger(); private: - static void DispatchDebugMessagesInMainThread(uv_async_t* handle); - static void DispatchDebugMessagesInMsgThread(); + void DoAcceptLoop(); - static uv_async_t dispatch_debug_messages_async_; + base::Thread thread_; + scoped_ptr server_socket_; + scoped_ptr accepted_socket_; + + base::WeakPtrFactory weak_factory_; DISALLOW_COPY_AND_ASSIGN(NodeDebugger); }; From 20f21b707bae737dcc1e3bc6ba9fd2a57bf2437d Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Thu, 4 Sep 2014 22:08:42 +0800 Subject: [PATCH 2/8] Start a TCP server in the debugger thread. --- atom/browser/node_debugger.cc | 46 ++++++++++++++++++++++++----------- atom/browser/node_debugger.h | 22 ++++++++++------- 2 files changed, 45 insertions(+), 23 deletions(-) diff --git a/atom/browser/node_debugger.cc b/atom/browser/node_debugger.cc index 007180e8bd6c..7c45bfa6e0f1 100644 --- a/atom/browser/node_debugger.cc +++ b/atom/browser/node_debugger.cc @@ -10,8 +10,7 @@ #include "base/bind.h" #include "base/command_line.h" #include "base/strings/string_number_conversions.h" -#include "net/socket/stream_socket.h" -#include "net/socket/tcp_server_socket.h" +#include "net/socket/tcp_listen_socket.h" #include "v8/include/v8-debug.h" namespace atom { @@ -39,23 +38,19 @@ NodeDebugger::NodeDebugger() if (!port_str.empty()) base::StringToInt(port_str, &port); - if (!thread_.Start()) { + // Start a new IO thread. + base::Thread::Options options; + options.message_loop_type = base::MessageLoop::TYPE_IO; + if (!thread_.StartWithOptions(options)) { LOG(ERROR) << "Unable to start debugger thread"; return; } - net::IPAddressNumber ip_number; - if (!net::ParseIPLiteralToNumber("127.0.0.1", &ip_number)) { - LOG(ERROR) << "Unable to convert ip address"; - return; - } - - server_socket_.reset(new net::TCPServerSocket(NULL, net::NetLog::Source())); - server_socket_->Listen(net::IPEndPoint(ip_number, port), 1); - + // Start the server in new IO thread. thread_.message_loop()->PostTask( FROM_HERE, - base::Bind(&NodeDebugger::DoAcceptLoop, weak_factory_.GetWeakPtr())); + base::Bind(&NodeDebugger::StartServer, weak_factory_.GetWeakPtr(), + port)); } } @@ -63,7 +58,30 @@ NodeDebugger::~NodeDebugger() { thread_.Stop(); } -void NodeDebugger::DoAcceptLoop() { +void NodeDebugger::StartServer(int port) { + server_ = net::TCPListenSocket::CreateAndListen("127.0.0.1", port, this); + if (!server_) { + LOG(ERROR) << "Cannot start debugger server"; + return; + } +} + +void NodeDebugger::DidAccept(net::StreamListenSocket* server, + scoped_ptr socket) { + // Only accept one session. + if (accepted_socket_) + return; + + accepted_socket_ = socket.Pass(); +} + +void NodeDebugger::DidRead(net::StreamListenSocket* socket, + const char* data, + int len) { +} + +void NodeDebugger::DidClose(net::StreamListenSocket* socket) { + accepted_socket_.reset(); } } // namespace atom diff --git a/atom/browser/node_debugger.h b/atom/browser/node_debugger.h index a0eb3a6b4632..818a8471302b 100644 --- a/atom/browser/node_debugger.h +++ b/atom/browser/node_debugger.h @@ -8,26 +8,30 @@ #include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" #include "base/threading/thread.h" - -namespace net { -class StreamSocket; -class TCPServerSocket; -} +#include "net/socket/stream_listen_socket.h" namespace atom { // Add support for node's "--debug" switch. -class NodeDebugger { +class NodeDebugger : public net::StreamListenSocket::Delegate { public: NodeDebugger(); virtual ~NodeDebugger(); private: - void DoAcceptLoop(); + void StartServer(int port); + + // net::StreamListenSocket::Delegate: + virtual void DidAccept(net::StreamListenSocket* server, + scoped_ptr socket) OVERRIDE; + virtual void DidRead(net::StreamListenSocket* socket, + const char* data, + int len) OVERRIDE; + virtual void DidClose(net::StreamListenSocket* socket) OVERRIDE; base::Thread thread_; - scoped_ptr server_socket_; - scoped_ptr accepted_socket_; + scoped_ptr server_; + scoped_ptr accepted_socket_; base::WeakPtrFactory weak_factory_; From ed7d430f2b5e591ec0343753f030c4da6c25744a Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 5 Sep 2014 09:39:28 +0800 Subject: [PATCH 3/8] Pass header sent from client. --- atom/browser/node_debugger.cc | 37 ++++++++++++++++++++++++++++++++++- atom/browser/node_debugger.h | 5 +++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/atom/browser/node_debugger.cc b/atom/browser/node_debugger.cc index 7c45bfa6e0f1..e554e97afb2f 100644 --- a/atom/browser/node_debugger.cc +++ b/atom/browser/node_debugger.cc @@ -17,6 +17,7 @@ namespace atom { NodeDebugger::NodeDebugger() : thread_("NodeDebugger"), + content_length_(-1), weak_factory_(this) { bool use_debug_agent = false; int port = 5858; @@ -69,8 +70,10 @@ void NodeDebugger::StartServer(int port) { void NodeDebugger::DidAccept(net::StreamListenSocket* server, scoped_ptr socket) { // Only accept one session. - if (accepted_socket_) + if (accepted_socket_) { + socket->Send(std::string("Remote debugging session already active"), true); return; + } accepted_socket_ = socket.Pass(); } @@ -78,6 +81,38 @@ void NodeDebugger::DidAccept(net::StreamListenSocket* server, void NodeDebugger::DidRead(net::StreamListenSocket* socket, const char* data, int len) { + buffer_.append(data, len); + + do { + if (buffer_.size() == 0) + return; + + // Read the "Content-Length" header. + if (content_length_ < 0) { + size_t pos = buffer_.find("\r\n\r\n"); + if (pos == std::string::npos) + return; + + // We can be sure that the header is "Content-Length: xxx\r\n". + std::string content_length = buffer_.substr(16, pos - 16); + if (!base::StringToInt(content_length, &content_length_)) { + DidClose(accepted_socket_.get()); + return; + } + + // Strip header from buffer. + buffer_ = buffer_.substr(pos + 4); + } + + // Read the message. + if (buffer_.size() >= static_cast(content_length_)) { + std::string message = buffer_.substr(0, content_length_); + buffer_ = buffer_.substr(content_length_); + + // Get ready for next message. + content_length_ = -1; + } + } while (true); } void NodeDebugger::DidClose(net::StreamListenSocket* socket) { diff --git a/atom/browser/node_debugger.h b/atom/browser/node_debugger.h index 818a8471302b..2c45e755fe94 100644 --- a/atom/browser/node_debugger.h +++ b/atom/browser/node_debugger.h @@ -5,6 +5,8 @@ #ifndef ATOM_BROWSER_NODE_DEBUGGER_H_ #define ATOM_BROWSER_NODE_DEBUGGER_H_ +#include + #include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" #include "base/threading/thread.h" @@ -33,6 +35,9 @@ class NodeDebugger : public net::StreamListenSocket::Delegate { scoped_ptr server_; scoped_ptr accepted_socket_; + std::string buffer_; + int content_length_; + base::WeakPtrFactory weak_factory_; DISALLOW_COPY_AND_ASSIGN(NodeDebugger); From 3795cc58b04a0e4f8267a0ad45ec8e2374d8b41f Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 5 Sep 2014 10:18:59 +0800 Subject: [PATCH 4/8] Pass debugger messages between V8 debugger. --- atom/browser/atom_browser_main_parts.cc | 7 ++- atom/browser/node_debugger.cc | 69 +++++++++++++++++++++++-- atom/browser/node_debugger.h | 12 ++++- 3 files changed, 81 insertions(+), 7 deletions(-) diff --git a/atom/browser/atom_browser_main_parts.cc b/atom/browser/atom_browser_main_parts.cc index f919e4e3ab80..890b57784e16 100644 --- a/atom/browser/atom_browser_main_parts.cc +++ b/atom/browser/atom_browser_main_parts.cc @@ -12,6 +12,7 @@ #include "atom/common/api/atom_bindings.h" #include "atom/common/node_bindings.h" #include "base/command_line.h" +#include "v8/include/v8-debug.h" #if defined(USE_X11) #include "chrome/browser/ui/libgtk2ui/gtk2_util.h" @@ -55,11 +56,15 @@ void AtomBrowserMainParts::PostEarlyInitialization() { node_bindings_->Initialize(); // Support the "--debug" switch. - node_debugger_.reset(new NodeDebugger); + node_debugger_.reset(new NodeDebugger(js_env_->isolate())); // Create the global environment. global_env = node_bindings_->CreateEnvironment(js_env_->context()); + // Make sure node can get correct environment when debugging. + if (node_debugger_->IsRunning()) + global_env->AssignToContext(v8::Debug::GetDebugContext()); + // Add atom-shell extended APIs. atom_bindings_->BindTo(js_env_->isolate(), global_env->process_object()); } diff --git a/atom/browser/node_debugger.cc b/atom/browser/node_debugger.cc index e554e97afb2f..68ab20bb924a 100644 --- a/atom/browser/node_debugger.cc +++ b/atom/browser/node_debugger.cc @@ -6,17 +6,29 @@ #include -#include "atom/common/atom_version.h" #include "base/bind.h" #include "base/command_line.h" #include "base/strings/string_number_conversions.h" +#include "base/strings/stringprintf.h" +#include "base/strings/utf_string_conversions.h" +#include "content/public/browser/browser_thread.h" #include "net/socket/tcp_listen_socket.h" -#include "v8/include/v8-debug.h" + +#include "atom/common/node_includes.h" namespace atom { -NodeDebugger::NodeDebugger() - : thread_("NodeDebugger"), +namespace { + +// NodeDebugger is stored in Isolate's data, slots 0, 1, 3 have already been +// taken by gin, blink and node, using 2 is a safe option for now. +const int kIsolateSlot = 2; + +} // namespace + +NodeDebugger::NodeDebugger(v8::Isolate* isolate) + : isolate_(isolate), + thread_("NodeDebugger"), content_length_(-1), weak_factory_(this) { bool use_debug_agent = false; @@ -39,6 +51,9 @@ NodeDebugger::NodeDebugger() if (!port_str.empty()) base::StringToInt(port_str, &port); + isolate_->SetData(kIsolateSlot, this); + v8::Debug::SetMessageHandler(DebugMessageHandler); + // Start a new IO thread. base::Thread::Options options; options.message_loop_type = base::MessageLoop::TYPE_IO; @@ -59,6 +74,10 @@ NodeDebugger::~NodeDebugger() { thread_.Stop(); } +bool NodeDebugger::IsRunning() const { + return thread_.IsRunning(); +} + void NodeDebugger::StartServer(int port) { server_ = net::TCPListenSocket::CreateAndListen("127.0.0.1", port, this); if (!server_) { @@ -67,6 +86,43 @@ void NodeDebugger::StartServer(int port) { } } +void NodeDebugger::CloseSession() { + accepted_socket_.reset(); +} + +void NodeDebugger::OnMessage(const std::string& message) { + if (message.find("\"type\":\"request\",\"command\":\"disconnect\"}") != + std::string::npos) + CloseSession(); + + base::string16 message16 = base::UTF8ToUTF16(message); + v8::Debug::SendCommand(isolate_, message16.data(), message16.size()); + + content::BrowserThread::PostTask( + content::BrowserThread::UI, FROM_HERE, + base::Bind(&v8::Debug::ProcessDebugMessages)); +} + +void NodeDebugger::SendMessage(const std::string& message) { + if (accepted_socket_) { + std::string header = base::StringPrintf("Content-Length: %d\r\n\r\n", + static_cast(message.size())); + accepted_socket_->Send(header); + accepted_socket_->Send(message); + } +} + +// static +void NodeDebugger::DebugMessageHandler(const v8::Debug::Message& message) { + NodeDebugger* self = static_cast( + message.GetIsolate()->GetData(kIsolateSlot)); + + if (self) { + std::string message8(*v8::String::Utf8Value(message.GetJSON())); + self->SendMessage(message8); + } +} + void NodeDebugger::DidAccept(net::StreamListenSocket* server, scoped_ptr socket) { // Only accept one session. @@ -109,6 +165,8 @@ void NodeDebugger::DidRead(net::StreamListenSocket* socket, std::string message = buffer_.substr(0, content_length_); buffer_ = buffer_.substr(content_length_); + OnMessage(message); + // Get ready for next message. content_length_ = -1; } @@ -116,7 +174,8 @@ void NodeDebugger::DidRead(net::StreamListenSocket* socket, } void NodeDebugger::DidClose(net::StreamListenSocket* socket) { - accepted_socket_.reset(); + // If we lost the connection, then simulate a disconnect msg: + OnMessage("{\"seq\":1,\"type\":\"request\",\"command\":\"disconnect\"}"); } } // namespace atom diff --git a/atom/browser/node_debugger.h b/atom/browser/node_debugger.h index 2c45e755fe94..bc2c392051d1 100644 --- a/atom/browser/node_debugger.h +++ b/atom/browser/node_debugger.h @@ -11,17 +11,25 @@ #include "base/memory/weak_ptr.h" #include "base/threading/thread.h" #include "net/socket/stream_listen_socket.h" +#include "v8/include/v8-debug.h" namespace atom { // Add support for node's "--debug" switch. class NodeDebugger : public net::StreamListenSocket::Delegate { public: - NodeDebugger(); + explicit NodeDebugger(v8::Isolate* isolate); virtual ~NodeDebugger(); + bool IsRunning() const; + private: void StartServer(int port); + void CloseSession(); + void OnMessage(const std::string& message); + void SendMessage(const std::string& message); + + static void DebugMessageHandler(const v8::Debug::Message& message); // net::StreamListenSocket::Delegate: virtual void DidAccept(net::StreamListenSocket* server, @@ -31,6 +39,8 @@ class NodeDebugger : public net::StreamListenSocket::Delegate { int len) OVERRIDE; virtual void DidClose(net::StreamListenSocket* socket) OVERRIDE; + v8::Isolate* isolate_; + base::Thread thread_; scoped_ptr server_; scoped_ptr accepted_socket_; From e3b6ea62d65389fd7c719efdc8da98debdcbb247 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 5 Sep 2014 10:52:47 +0800 Subject: [PATCH 5/8] Send connect message if client is connected. --- atom/browser/node_debugger.cc | 17 +++++++++++++++-- atom/browser/node_debugger.h | 1 + 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/atom/browser/node_debugger.cc b/atom/browser/node_debugger.cc index 68ab20bb924a..55cc3e68f24f 100644 --- a/atom/browser/node_debugger.cc +++ b/atom/browser/node_debugger.cc @@ -24,6 +24,8 @@ namespace { // taken by gin, blink and node, using 2 is a safe option for now. const int kIsolateSlot = 2; +const char* kContentLength = "Content-Length"; + } // namespace NodeDebugger::NodeDebugger(v8::Isolate* isolate) @@ -105,13 +107,23 @@ void NodeDebugger::OnMessage(const std::string& message) { void NodeDebugger::SendMessage(const std::string& message) { if (accepted_socket_) { - std::string header = base::StringPrintf("Content-Length: %d\r\n\r\n", - static_cast(message.size())); + std::string header = base::StringPrintf( + "%s: %d\r\n\r\n", kContentLength, static_cast(message.size())); accepted_socket_->Send(header); accepted_socket_->Send(message); } } +void NodeDebugger::SendConnectMessage() { + accepted_socket_->Send(base::StringPrintf( + "Type: connect\r\n" + "V8-Version: %s\r\n" + "Protocol-Version: 1\r\n" + "Embedding-Host: %s\r\n" + "%s: 0\r\n", + v8::V8::GetVersion(), "Atom-Shell", kContentLength), true); +} + // static void NodeDebugger::DebugMessageHandler(const v8::Debug::Message& message) { NodeDebugger* self = static_cast( @@ -132,6 +144,7 @@ void NodeDebugger::DidAccept(net::StreamListenSocket* server, } accepted_socket_ = socket.Pass(); + SendConnectMessage(); } void NodeDebugger::DidRead(net::StreamListenSocket* socket, diff --git a/atom/browser/node_debugger.h b/atom/browser/node_debugger.h index bc2c392051d1..39273fa2dc2e 100644 --- a/atom/browser/node_debugger.h +++ b/atom/browser/node_debugger.h @@ -28,6 +28,7 @@ class NodeDebugger : public net::StreamListenSocket::Delegate { void CloseSession(); void OnMessage(const std::string& message); void SendMessage(const std::string& message); + void SendConnectMessage(); static void DebugMessageHandler(const v8::Debug::Message& message); From 222f8e102819e0e2074a6981fcbef22e84a6e19c Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 5 Sep 2014 10:59:29 +0800 Subject: [PATCH 6/8] Make --debug-brk work. --- atom/browser/node_debugger.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/atom/browser/node_debugger.cc b/atom/browser/node_debugger.cc index 55cc3e68f24f..5d7f1ca1a01a 100644 --- a/atom/browser/node_debugger.cc +++ b/atom/browser/node_debugger.cc @@ -56,6 +56,9 @@ NodeDebugger::NodeDebugger(v8::Isolate* isolate) isolate_->SetData(kIsolateSlot, this); v8::Debug::SetMessageHandler(DebugMessageHandler); + if (wait_for_connection) + v8::Debug::DebugBreak(isolate_); + // Start a new IO thread. base::Thread::Options options; options.message_loop_type = base::MessageLoop::TYPE_IO; From fadfd7492352ccea0963760bd9107ebfd91fda20 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 5 Sep 2014 11:44:30 +0800 Subject: [PATCH 7/8] win: Fix compilation error. --- atom/browser/node_debugger.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/atom/browser/node_debugger.cc b/atom/browser/node_debugger.cc index 5d7f1ca1a01a..a8a0c8ef9d09 100644 --- a/atom/browser/node_debugger.cc +++ b/atom/browser/node_debugger.cc @@ -101,7 +101,9 @@ void NodeDebugger::OnMessage(const std::string& message) { CloseSession(); base::string16 message16 = base::UTF8ToUTF16(message); - v8::Debug::SendCommand(isolate_, message16.data(), message16.size()); + v8::Debug::SendCommand( + isolate_, + reinterpret_cast(message16.data()), message16.size()); content::BrowserThread::PostTask( content::BrowserThread::UI, FROM_HERE, From 3afbb66b9287f0e30b993f8555e750bc6550f57c Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 5 Sep 2014 11:51:45 +0800 Subject: [PATCH 8/8] Send message to client in debugger thread. --- atom/browser/node_debugger.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/atom/browser/node_debugger.cc b/atom/browser/node_debugger.cc index a8a0c8ef9d09..9297aa5574f4 100644 --- a/atom/browser/node_debugger.cc +++ b/atom/browser/node_debugger.cc @@ -136,7 +136,10 @@ void NodeDebugger::DebugMessageHandler(const v8::Debug::Message& message) { if (self) { std::string message8(*v8::String::Utf8Value(message.GetJSON())); - self->SendMessage(message8); + self->thread_.message_loop()->PostTask( + FROM_HERE, + base::Bind(&NodeDebugger::SendMessage, self->weak_factory_.GetWeakPtr(), + message8)); } }