Skip to content

Commit

Permalink
inspector: Do not crash if the port is n/a
Browse files Browse the repository at this point in the history 
Node process will no longer terminate with an assertion if the
inspector port is not available.

PR-URL: #7874
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: jasnell - James M Snell <jasnell@gmail.com>

Conflicts:
	src/node.cc
  • Loading branch information
@cjihrig
Eugene Ostroukhov authored and cjihrig committed Aug 10, 2016
1 parent 96d15e2 commit b75ca50
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 47 deletions.
5 changes: 3 additions & 2 deletions doc/api/process.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1660,8 +1660,9 @@ cases:
source code internal in Node.js's bootstrapping process threw an error
when the bootstrapping function was called. This is extremely rare,
and generally can only happen during development of Node.js itself.
* `12` **Invalid Debug Argument** - The `--debug` and/or `--debug-brk`
options were set, but an invalid port number was chosen.
* `12` **Invalid Debug Argument** - The `--debug`, `--inspect` and/or
`--debug-brk` options were set, but the port number chosen was invalid
or unavailable.
* `>128` **Signal Exits** - If Node.js receives a fatal signal such as
`SIGKILL` or `SIGHUP`, then its exit code will be `128` plus the
value of the signal code. This is a standard Unix practice, since
Expand Down
80 changes: 41 additions & 39 deletions src/inspector_agent.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,16 +164,17 @@ class AgentImpl {
~AgentImpl();

// Start the inspector agent thread
void Start(v8::Platform* platform, int port, bool wait);
bool Start(v8::Platform* platform, int port, bool wait);
// Stop the inspector agent
void Stop();

bool IsStarted();
bool IsConnected() { return connected_; }
bool IsConnected() { return state_ == State::kConnected; }
void WaitForDisconnect();

private:
using MessageQueue = std::vector<std::pair<int, String16>>;
enum class State { kNew, kAccepting, kConnected, kDone, kError };

static void ThreadCbIO(void* agent);
static void OnSocketConnectionIO(uv_stream_t* server, int status);
Expand All @@ -194,6 +195,7 @@ class AgentImpl {
const String16& message);
void SwapBehindLock(MessageQueue* vector1, MessageQueue* vector2);
void PostIncomingMessage(const String16& message);
State ToState(State state);

uv_sem_t start_sem_;
ConditionVariable pause_cond_;
Expand All @@ -204,8 +206,8 @@ class AgentImpl {

int port_;
bool wait_;
bool connected_;
bool shutting_down_;
State state_;
node::Environment* parent_env_;

uv_async_t data_written_;
Expand Down Expand Up @@ -313,8 +315,8 @@ class V8NodeInspector : public blink::V8Inspector {

AgentImpl::AgentImpl(Environment* env) : port_(0),
wait_(false),
connected_(false),
shutting_down_(false),
state_(State::kNew),
parent_env_(env),
client_socket_(nullptr),
inspector_(nullptr),
Expand All @@ -333,17 +335,11 @@ AgentImpl::~AgentImpl() {
uv_close(reinterpret_cast<uv_handle_t*>(&data_written_), nullptr);
}

void AgentImpl::Start(v8::Platform* platform, int port, bool wait) {
bool AgentImpl::Start(v8::Platform* platform, int port, bool wait) {
auto env = parent_env_;
inspector_ = new V8NodeInspector(this, env, platform);

int err;

platform_ = platform;

err = uv_loop_init(&child_loop_);
CHECK_EQ(err, 0);
err = uv_async_init(env->event_loop(), &data_written_, nullptr);
int err = uv_async_init(env->event_loop(), &data_written_, nullptr);
CHECK_EQ(err, 0);

uv_unref(reinterpret_cast<uv_handle_t*>(&data_written_));
Expand All @@ -355,21 +351,20 @@ void AgentImpl::Start(v8::Platform* platform, int port, bool wait) {
CHECK_EQ(err, 0);
uv_sem_wait(&start_sem_);

if (state_ == State::kError) {
Stop();
return false;
}
state_ = State::kAccepting;
if (wait) {
DispatchMessages();
}
return true;
}

void AgentImpl::Stop() {
// TODO(repenaxa): hop on the right thread.
DisconnectAndDisposeIO(client_socket_);
int err = uv_thread_join(&thread_);
CHECK_EQ(err, 0);

uv_run(&child_loop_, UV_RUN_NOWAIT);

err = uv_loop_close(&child_loop_);
CHECK_EQ(err, 0);
delete inspector_;
}

Expand Down Expand Up @@ -428,7 +423,6 @@ void AgentImpl::OnRemoteDataIO(inspector_socket_t* socket,
Mutex::ScopedLock scoped_lock(pause_lock_);
if (read > 0) {
String16 str = String16::fromUTF8(buf->base, read);
PostIncomingMessage(str);
// TODO(pfeldman): Instead of blocking execution while debugger
// engages, node should wait for the run callback from the remote client
// and initiate its startup. This is a change to node.cc that should be
Expand All @@ -437,11 +431,7 @@ void AgentImpl::OnRemoteDataIO(inspector_socket_t* socket,
wait_ = false;
uv_sem_post(&start_sem_);
}

platform_->CallOnForegroundThread(parent_env_->isolate(),
new DispatchOnInspectorBackendTask(this));
parent_env_->isolate()->RequestInterrupt(InterruptCallback, this);
uv_async_send(&data_written_);
PostIncomingMessage(str);
} else if (read <= 0) {
// EOF
if (client_socket_ == socket) {
Expand Down Expand Up @@ -476,8 +466,10 @@ void AgentImpl::WriteCbIO(uv_async_t* async) {
void AgentImpl::WorkerRunIO() {
sockaddr_in addr;
uv_tcp_t server;
int err = uv_async_init(&child_loop_, &io_thread_req_, AgentImpl::WriteCbIO);
CHECK_EQ(0, err);
int err = uv_loop_init(&child_loop_);
CHECK_EQ(err, 0);
err = uv_async_init(&child_loop_, &io_thread_req_, AgentImpl::WriteCbIO);
CHECK_EQ(err, 0);
io_thread_req_.data = this;
uv_tcp_init(&child_loop_, &server);
uv_ip4_addr("0.0.0.0", port_, &addr);
Expand All @@ -488,19 +480,26 @@ void AgentImpl::WorkerRunIO() {
err = uv_listen(reinterpret_cast<uv_stream_t*>(&server), 1,
OnSocketConnectionIO);
}
if (err == 0) {
PrintDebuggerReadyMessage(port_);
} else {
if (err != 0) {
fprintf(stderr, "Unable to open devtools socket: %s\n", uv_strerror(err));
ABORT();
state_ = State::kError; // Safe, main thread is waiting on semaphore
uv_close(reinterpret_cast<uv_handle_t*>(&io_thread_req_), nullptr);
uv_close(reinterpret_cast<uv_handle_t*>(&server), nullptr);
uv_loop_close(&child_loop_);
uv_sem_post(&start_sem_);
return;
}
PrintDebuggerReadyMessage(port_);
if (!wait_) {
uv_sem_post(&start_sem_);
}
uv_run(&child_loop_, UV_RUN_DEFAULT);
uv_close(reinterpret_cast<uv_handle_t*>(&io_thread_req_), nullptr);
uv_close(reinterpret_cast<uv_handle_t*>(&server), nullptr);
uv_run(&child_loop_, UV_RUN_DEFAULT);
DisconnectAndDisposeIO(client_socket_);
uv_run(&child_loop_, UV_RUN_NOWAIT);
err = uv_loop_close(&child_loop_);
CHECK_EQ(err, 0);
}

void AgentImpl::AppendMessage(MessageQueue* queue, int session_id,
Expand Down Expand Up @@ -543,16 +542,19 @@ void AgentImpl::DispatchMessages() {
for (const MessageQueue::value_type& pair : tasks) {
const String16& message = pair.second;
if (message == TAG_CONNECT) {
CHECK_EQ(false, connected_);
CHECK_EQ(State::kAccepting, state_);
backend_session_id_++;
connected_ = true;
state_ = State::kConnected;
fprintf(stderr, "Debugger attached.\n");
inspector_->connectFrontend(new ChannelImpl(this));
} else if (message == TAG_DISCONNECT) {
CHECK(connected_);
connected_ = false;
if (!shutting_down_)
CHECK_EQ(State::kConnected, state_);
if (shutting_down_) {
state_ = State::kDone;
} else {
PrintDebuggerReadyMessage(port_);
state_ = State::kAccepting;
}
inspector_->quitMessageLoopOnPause();
inspector_->disconnectFrontend();
} else {
Expand All @@ -576,8 +578,8 @@ Agent::~Agent() {
delete impl;
}

void Agent::Start(v8::Platform* platform, int port, bool wait) {
impl->Start(platform, port, wait);
bool Agent::Start(v8::Platform* platform, int port, bool wait) {
return impl->Start(platform, port, wait);
}

void Agent::Stop() {
Expand Down
2 changes: 1 addition & 1 deletion src/inspector_agent.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class Agent {
~Agent();

// Start the inspector agent thread
void Start(v8::Platform* platform, int port, bool wait);
bool Start(v8::Platform* platform, int port, bool wait);
// Stop the inspector agent
void Stop();

Expand Down
15 changes: 10 additions & 5 deletions src/node.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,11 @@ static struct {
platform_ = nullptr;
}

void StartInspector(Environment *env, int port, bool wait) {
bool StartInspector(Environment *env, int port, bool wait) {
#if HAVE_INSPECTOR
env->inspector_agent()->Start(platform_, port, wait);
return env->inspector_agent()->Start(platform_, port, wait);
#else
return true;
#endif // HAVE_INSPECTOR
}

Expand Down Expand Up @@ -3836,8 +3838,7 @@ static void DispatchMessagesDebugAgentCallback(Environment* env) {
static void StartDebug(Environment* env, bool wait) {
CHECK(!debugger_running);
if (use_inspector) {
v8_platform.StartInspector(env, inspector_port, wait);
debugger_running = true;
debugger_running = v8_platform.StartInspector(env, inspector_port, wait);
} else {
env->debugger_agent()->set_dispatch_handler(
DispatchMessagesDebugAgentCallback);
Expand Down Expand Up @@ -4547,8 +4548,12 @@ static void StartNodeInstance(void* arg) {
ShouldAbortOnUncaughtException);

// Start debug agent when argv has --debug
if (instance_data->use_debug_agent())
if (instance_data->use_debug_agent()) {
StartDebug(env, debug_wait_connect);
if (use_inspector && !debugger_running) {
exit(12);
}
}

{
Environment::AsyncCallbackScope callback_scope(env);
Expand Down

0 comments on commit b75ca50

Please sign in to comment.