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

inspector: no crash when WS server can't start #10878

Merged
merged 1 commit into from
Jan 20, 2017
Merged

inspector: no crash when WS server can't start #10878

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
5 changes: 3 additions & 2 deletions src/inspector_agent.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,9 +556,10 @@ void AgentImpl::WorkerRunIO() {
}
InspectorAgentDelegate delegate(this, script_path, script_name_, wait_);
delegate_ = &delegate;
InspectorSocketServer server(&delegate, options_.port());
InspectorSocketServer server(&delegate,
options_.host_name(),
options_.port());
if (!server.Start(&child_loop_)) {
fprintf(stderr, "Unable to open devtools socket: %s\n", uv_strerror(err));
state_ = State::kError; // Safe, main thread is waiting on semaphore
uv_close(reinterpret_cast<uv_handle_t*>(&io_thread_req_), nullptr);
uv_loop_close(&child_loop_);
Expand Down
24 changes: 15 additions & 9 deletions src/inspector_socket_server.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ void Escape(std::string* string) {
}
}

std::string GetWsUrl(int port, const std::string& id) {
std::string GetWsUrl(const std::string& host, int port, const std::string& id) {
char buf[1024];
snprintf(buf, sizeof(buf), "127.0.0.1:%d/%s", port, id.c_str());
snprintf(buf, sizeof(buf), "%s:%d/%s", host.c_str(), port, id.c_str());
return buf;
}

Expand Down Expand Up @@ -74,7 +74,8 @@ void OnBufferAlloc(uv_handle_t* handle, size_t len, uv_buf_t* buf) {
buf->len = len;
}

void PrintDebuggerReadyMessage(int port,
void PrintDebuggerReadyMessage(const std::string& host,
int port,
const std::vector<std::string>& ids,
FILE* out) {
if (out == NULL) {
Expand All @@ -92,7 +93,8 @@ void PrintDebuggerReadyMessage(int port,
for (const std::string& id : ids) {
fprintf(out,
" chrome-devtools://devtools/bundled/inspector.html?"
"experiments=true&v8only=true&ws=%s\n", GetWsUrl(port, id).c_str());
"experiments=true&v8only=true&ws=%s\n",
GetWsUrl(host, port, id).c_str());
}
fflush(out);
}
Expand Down Expand Up @@ -229,9 +231,11 @@ class SocketSession {
};

InspectorSocketServer::InspectorSocketServer(SocketServerDelegate* delegate,
const std::string& host,
int port,
FILE* out) : loop_(nullptr),
delegate_(delegate),
host_(host),
port_(port),
server_(uv_tcp_t()),
closer_(nullptr),
Expand Down Expand Up @@ -284,7 +288,7 @@ void InspectorSocketServer::SessionTerminated(int session_id) {
delegate_->EndSession(session_id);
if (connected_sessions_.empty() &&
uv_is_active(reinterpret_cast<uv_handle_t*>(&server_))) {
PrintDebuggerReadyMessage(port_, delegate_->GetTargetIds(), out_);
PrintDebuggerReadyMessage(host_, port_, delegate_->GetTargetIds(), out_);
}
}

Expand Down Expand Up @@ -337,7 +341,7 @@ void InspectorSocketServer::SendListResponse(InspectorSocket* socket) {
}
}
if (!connected) {
std::string address = GetWsUrl(port_, id);
std::string address = GetWsUrl(host_, port_, id);
std::ostringstream frontend_url;
frontend_url << "chrome-devtools://devtools/bundled";
frontend_url << "/inspector.html?experiments=true&v8only=true&ws=";
Expand All @@ -353,7 +357,7 @@ bool InspectorSocketServer::Start(uv_loop_t* loop) {
loop_ = loop;
sockaddr_in addr;
uv_tcp_init(loop_, &server_);
uv_ip4_addr("0.0.0.0", port_, &addr);
uv_ip4_addr(host_.c_str(), port_, &addr);
int err = uv_tcp_bind(&server_,
reinterpret_cast<const struct sockaddr*>(&addr), 0);
if (err == 0)
Expand All @@ -363,11 +367,13 @@ bool InspectorSocketServer::Start(uv_loop_t* loop) {
SocketConnectedCallback);
}
if (err == 0 && connected_sessions_.empty()) {
PrintDebuggerReadyMessage(port_, delegate_->GetTargetIds(), out_);
PrintDebuggerReadyMessage(host_, port_, delegate_->GetTargetIds(), out_);
}
if (err != 0 && connected_sessions_.empty()) {
if (out_ != NULL) {
fprintf(out_, "Unable to open devtools socket: %s\n", uv_strerror(err));
fprintf(out_, "Starting inspector on %s:%d failed: %s\n",
host_.c_str(), port_, uv_strerror(err));
fflush(out_);
}
uv_close(reinterpret_cast<uv_handle_t*>(&server_), nullptr);
return false;
Expand Down
2 changes: 2 additions & 0 deletions src/inspector_socket_server.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class InspectorSocketServer {
public:
using ServerCallback = void (*)(InspectorSocketServer*);
InspectorSocketServer(SocketServerDelegate* delegate,
const std::string& host,
int port,
FILE* out = stderr);
bool Start(uv_loop_t* loop);
Expand Down Expand Up @@ -65,6 +66,7 @@ class InspectorSocketServer {

uv_loop_t* loop_;
SocketServerDelegate* const delegate_;
const std::string host_;
int port_;
std::string path_;
uv_tcp_t server_;
Expand Down
2 changes: 1 addition & 1 deletion src/node.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4362,7 +4362,7 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data,
if (debug_enabled) {
const char* path = argc > 1 ? argv[1] : nullptr;
StartDebug(&env, path, debug_options);
if (debug_options.debugger_enabled() && !debugger_running)
if (!debugger_running)
return 12; // Signal internal error.
}

Expand Down
22 changes: 12 additions & 10 deletions test/cctest/test_inspector_socket_server.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

static uv_loop_t loop;

static const char HOST[] = "127.0.0.1";

static const char CLIENT_CLOSE_FRAME[] = "\x88\x80\x2D\x0E\x1E\xFA";
static const char SERVER_CLOSE_FRAME[] = "\x88\x00";

Expand Down Expand Up @@ -249,7 +251,7 @@ class SocketWrapper {
}

static void Connected_(uv_connect_t* connect, int status) {
EXPECT_EQ(0, status);
EXPECT_EQ(0, status) << "Unable to connect: " << uv_strerror(status);
SocketWrapper* wrapper =
node::ContainerOf(&SocketWrapper::connect_, connect);
wrapper->connected_ = true;
Expand Down Expand Up @@ -301,7 +303,7 @@ class ServerHolder {
template <typename Delegate>
ServerHolder(Delegate* delegate, int port, FILE* out = NULL)
: closed(false), paused(false), sessions_terminated(false),
server_(delegate, port, out) {
server_(delegate, HOST, port, out) {
delegate->Connect(&server_);
}

Expand Down Expand Up @@ -362,7 +364,7 @@ class ServerDelegateNoTargets : public SocketServerDelegate {
static void TestHttpRequest(int port, const std::string& path,
const std::string& expected_body) {
SocketWrapper socket(&loop);
socket.Connect("0.0.0.0", port);
socket.Connect(HOST, port);
socket.TestHttpRequest(path, expected_body);
socket.Close();
}
Expand All @@ -385,7 +387,7 @@ TEST_F(InspectorSocketServerTest, InspectorSessions) {

SocketWrapper well_behaved_socket(&loop);
// Regular connection
well_behaved_socket.Connect("0.0.0.0", server.port());
well_behaved_socket.Connect(HOST, server.port());
well_behaved_socket.Write(WsHandshakeRequest(MAIN_TARGET_ID));
well_behaved_socket.Expect(WS_HANDSHAKE_RESPONSE);

Expand All @@ -408,7 +410,7 @@ TEST_F(InspectorSocketServerTest, InspectorSessions) {

// Declined connection
SocketWrapper declined_target_socket(&loop);
declined_target_socket.Connect("127.0.0.1", server.port());
declined_target_socket.Connect(HOST, server.port());
declined_target_socket.Write(WsHandshakeRequest(UNCONNECTABLE_TARGET_ID));
declined_target_socket.Expect("HTTP/1.0 400 Bad Request");
declined_target_socket.ExpectEOF();
Expand All @@ -417,7 +419,7 @@ TEST_F(InspectorSocketServerTest, InspectorSessions) {

// Bogus target - start session callback should not even be invoked
SocketWrapper bogus_target_socket(&loop);
bogus_target_socket.Connect("127.0.0.1", server.port());
bogus_target_socket.Connect(HOST, server.port());
bogus_target_socket.Write(WsHandshakeRequest("bogus_target"));
bogus_target_socket.Expect("HTTP/1.0 400 Bad Request");
bogus_target_socket.ExpectEOF();
Expand All @@ -426,7 +428,7 @@ TEST_F(InspectorSocketServerTest, InspectorSessions) {

// Drop connection (no proper close frames)
SocketWrapper dropped_connection_socket(&loop);
dropped_connection_socket.Connect("127.0.0.1", server.port());
dropped_connection_socket.Connect(HOST, server.port());
dropped_connection_socket.Write(WsHandshakeRequest(MAIN_TARGET_ID));
dropped_connection_socket.Expect(WS_HANDSHAKE_RESPONSE);

Expand All @@ -440,7 +442,7 @@ TEST_F(InspectorSocketServerTest, InspectorSessions) {

// Reconnect regular connection
SocketWrapper stays_till_termination_socket(&loop);
stays_till_termination_socket.Connect("127.0.0.1", server.port());
stays_till_termination_socket.Connect(HOST, server.port());
stays_till_termination_socket.Write(WsHandshakeRequest(MAIN_TARGET_ID));
stays_till_termination_socket.Expect(WS_HANDSHAKE_RESPONSE);

Expand Down Expand Up @@ -484,7 +486,7 @@ TEST_F(InspectorSocketServerTest, ServerWithoutTargets) {

// Declined connection
SocketWrapper socket(&loop);
socket.Connect("0.0.0.0", server.port());
socket.Connect(HOST, server.port());
socket.Write(WsHandshakeRequest(UNCONNECTABLE_TARGET_ID));
socket.Expect("HTTP/1.0 400 Bad Request");
socket.ExpectEOF();
Expand Down Expand Up @@ -512,7 +514,7 @@ TEST_F(InspectorSocketServerTest, StoppingServerDoesNotKillConnections) {
ServerHolder server(&delegate, 0);
ASSERT_TRUE(server->Start(&loop));
SocketWrapper socket1(&loop);
socket1.Connect("0.0.0.0", server.port());
socket1.Connect(HOST, server.port());
socket1.TestHttpRequest("/json/list", "[ ]");
server->Stop(ServerHolder::CloseCallback);
SPIN_WHILE(!server.closed);
Expand Down