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

Jump to bottom

inspector: Do not crash if the port is n/a #7874

Closed
wants to merge 1 commit into from
Closed

inspector: Do not crash if the port is n/a #7874

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 doc/api/process.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1643,8 +1643,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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,16 +165,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 @@ -195,6 +196,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 @@ -205,8 +207,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 @@ -314,8 +316,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 @@ -334,17 +336,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 @@ -356,21 +352,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 @@ -429,7 +424,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 @@ -438,11 +432,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 @@ -477,8 +467,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 @@ -489,19 +481,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 @@ -544,16 +543,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 @@ -577,8 +579,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
Open in desktop
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
Open in desktop
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 @@ -3756,8 +3758,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 @@ -4383,8 +4384,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