Skip to content

Commit 77d5e6f

Browse files
Eugene Ostroukhovjasnell
Eugene Ostroukhov
authored and
jasnell
committed
inspector: fix process._debugEnd() for inspector
This change ensures that the WebSocket server can be stopped (and restarted if needed) buy calling process._debugEnd. PR-URL: #12777 Fixes: #12559 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>
1 parent e6c03c7 commit 77d5e6f

11 files changed

+385
-148
lines changed

‎src/inspector_agent.cc‎

Lines changed: 51 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,18 @@ using v8_inspector::V8Inspector;
3838
static uv_sem_t inspector_io_thread_semaphore;
3939
static uv_async_t start_inspector_thread_async;
4040

41+
class StartIoTask : public v8::Task {
42+
public:
43+
explicit StartIoTask(Agent* agent) : agent(agent) {}
44+
45+
void Run() override {
46+
agent->StartIoThread(false);
47+
}
48+
49+
private:
50+
Agent* agent;
51+
};
52+
4153
std::unique_ptr<StringBuffer> ToProtocolString(Isolate* isolate,
4254
Local<Value> value) {
4355
TwoByteValue buffer(isolate, value);
@@ -46,9 +58,14 @@ std::unique_ptr<StringBuffer> ToProtocolString(Isolate* isolate,
4658

4759
// Called from the main thread.
4860
void StartInspectorIoThreadAsyncCallback(uv_async_t* handle) {
49-
static_cast<Agent*>(handle->data)->StartIoThread();
61+
static_cast<Agent*>(handle->data)->StartIoThread(false);
62+
}
63+
64+
void StartIoCallback(Isolate* isolate, void* agent) {
65+
static_cast<Agent*>(agent)->StartIoThread(false);
5066
}
5167

68+
5269
#ifdef __POSIX__
5370
static void EnableInspectorIOThreadSignalHandler(int signo) {
5471
uv_sem_post(&inspector_io_thread_semaphore);
@@ -57,7 +74,9 @@ static void EnableInspectorIOThreadSignalHandler(int signo) {
5774
inline void* InspectorIoThreadSignalThreadMain(void* unused) {
5875
for (;;) {
5976
uv_sem_wait(&inspector_io_thread_semaphore);
60-
uv_async_send(&start_inspector_thread_async);
77+
Agent* agent = static_cast<Agent*>(start_inspector_thread_async.data);
78+
if (agent != nullptr)
79+
agent->RequestIoStart();
6180
}
6281
return nullptr;
6382
}
@@ -103,7 +122,9 @@ static int RegisterDebugSignalHandler() {
103122

104123
#ifdef _WIN32
105124
DWORD WINAPI EnableDebugThreadProc(void* arg) {
106-
uv_async_send(&start_inspector_thread_async);
125+
Agent* agent = static_cast<Agent*>(start_inspector_thread_async.data);
126+
if (agent != nullptr)
127+
agent->RequestIoStart();
107128
return 0;
108129
}
109130

@@ -387,29 +408,29 @@ bool Agent::Start(v8::Platform* platform, const char* path,
387408
new NodeInspectorClient(parent_env_, platform));
388409
inspector_->contextCreated(parent_env_->context(), "Node.js Main Context");
389410
platform_ = platform;
411+
CHECK_EQ(0, uv_async_init(uv_default_loop(),
412+
&start_inspector_thread_async,
413+
StartInspectorIoThreadAsyncCallback));
414+
start_inspector_thread_async.data = this;
415+
uv_unref(reinterpret_cast<uv_handle_t*>(&start_inspector_thread_async));
416+
417+
RegisterDebugSignalHandler();
390418
if (options.inspector_enabled()) {
391-
return StartIoThread();
392-
} else {
393-
CHECK_EQ(0, uv_async_init(uv_default_loop(),
394-
&start_inspector_thread_async,
395-
StartInspectorIoThreadAsyncCallback));
396-
start_inspector_thread_async.data = this;
397-
uv_unref(reinterpret_cast<uv_handle_t*>(&start_inspector_thread_async));
398-
399-
RegisterDebugSignalHandler();
400-
return true;
419+
return StartIoThread(options.wait_for_connect());
401420
}
421+
return true;
402422
}
403423

404-
bool Agent::StartIoThread() {
424+
bool Agent::StartIoThread(bool wait_for_connect) {
405425
if (io_ != nullptr)
406426
return true;
407427

408428
CHECK_NE(inspector_, nullptr);
409429

410430
enabled_ = true;
411431
io_ = std::unique_ptr<InspectorIo>(
412-
new InspectorIo(parent_env_, platform_, path_, debug_options_));
432+
new InspectorIo(parent_env_, platform_, path_, debug_options_,
433+
wait_for_connect));
413434
if (!io_->Start()) {
414435
inspector_.reset();
415436
return false;
@@ -440,8 +461,10 @@ bool Agent::StartIoThread() {
440461
}
441462

442463
void Agent::Stop() {
443-
if (io_ != nullptr)
464+
if (io_ != nullptr) {
444465
io_->Stop();
466+
io_.reset();
467+
}
445468
}
446469

447470
void Agent::Connect(InspectorSessionDelegate* delegate) {
@@ -502,6 +525,18 @@ void Agent::InitJSBindings(Local<Object> target, Local<Value> unused,
502525
if (agent->debug_options_.wait_for_connect())
503526
env->SetMethod(target, "callAndPauseOnStart", CallAndPauseOnStart);
504527
}
528+
529+
void Agent::RequestIoStart() {
530+
// We need to attempt to interrupt V8 flow (in case Node is running
531+
// continuous JS code) and to wake up libuv thread (in case Node is wating
532+
// for IO events)
533+
uv_async_send(&start_inspector_thread_async);
534+
v8::Isolate* isolate = parent_env_->isolate();
535+
platform_->CallOnForegroundThread(isolate, new StartIoTask(this));
536+
isolate->RequestInterrupt(StartIoCallback, this);
537+
uv_async_send(&start_inspector_thread_async);
538+
}
539+
505540
} // namespace inspector
506541
} // namespace node
507542

‎src/inspector_agent.h‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ class Agent {
5151

5252
bool Start(v8::Platform* platform, const char* path,
5353
const DebugOptions& options);
54-
bool StartIoThread();
5554
void Stop();
5655

5756
bool IsStarted();
@@ -72,6 +71,13 @@ class Agent {
7271
v8::Local<v8::Context> context,
7372
void* priv);
7473

74+
bool StartIoThread(bool wait_for_connect);
75+
InspectorIo* io() {
76+
return io_.get();
77+
}
78+
// Can be called from any thread
79+
void RequestIoStart();
80+
7581
private:
7682
node::Environment* parent_env_;
7783
std::unique_ptr<NodeInspectorClient> inspector_;

‎src/inspector_io.cc‎

Lines changed: 64 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
namespace node {
2121
namespace inspector {
2222
namespace {
23+
using AsyncAndAgent = std::pair<uv_async_t, Agent*>;
2324
using v8_inspector::StringBuffer;
2425
using v8_inspector::StringView;
2526

@@ -96,6 +97,12 @@ int CloseAsyncAndLoop(uv_async_t* async) {
9697
return uv_loop_close(async->loop);
9798
}
9899

100+
void ReleasePairOnAsyncClose(uv_handle_t* async) {
101+
AsyncAndAgent* pair = node::ContainerOf(&AsyncAndAgent::first,
102+
reinterpret_cast<uv_async_t*>(async));
103+
delete pair;
104+
}
105+
99106
} // namespace
100107

101108
std::unique_ptr<StringBuffer> Utf8ToStringView(const std::string& message) {
@@ -127,6 +134,9 @@ class InspectorIoDelegate: public node::inspector::SocketServerDelegate {
127134
std::string GetTargetTitle(const std::string& id) override;
128135
std::string GetTargetUrl(const std::string& id) override;
129136
bool IsConnected() { return connected_; }
137+
void ServerDone() override {
138+
io_->ServerDone();
139+
}
130140
private:
131141
InspectorIo* io_;
132142
bool connected_;
@@ -137,53 +147,70 @@ class InspectorIoDelegate: public node::inspector::SocketServerDelegate {
137147
bool waiting_;
138148
};
139149

140-
void InterruptCallback(v8::Isolate*, void* io) {
141-
static_cast<InspectorIo*>(io)->DispatchMessages();
150+
void InterruptCallback(v8::Isolate*, void* agent) {
151+
InspectorIo* io = static_cast<Agent*>(agent)->io();
152+
if (io != nullptr)
153+
io->DispatchMessages();
142154
}
143155

144-
class DispatchOnInspectorBackendTask : public v8::Task {
156+
class DispatchMessagesTask : public v8::Task {
145157
public:
146-
explicit DispatchOnInspectorBackendTask(InspectorIo* io) : io_(io) {}
158+
explicit DispatchMessagesTask(Agent* agent) : agent_(agent) {}
147159

148160
void Run() override {
149-
io_->DispatchMessages();
161+
InspectorIo* io = agent_->io();
162+
if (io != nullptr)
163+
io->DispatchMessages();
150164
}
151165

152166
private:
153-
InspectorIo* io_;
167+
Agent* agent_;
154168
};
155169

156170
InspectorIo::InspectorIo(Environment* env, v8::Platform* platform,
157-
const std::string& path, const DebugOptions& options)
171+
const std::string& path, const DebugOptions& options,
172+
bool wait_for_connect)
158173
: options_(options), thread_(), delegate_(nullptr),
159-
shutting_down_(false), state_(State::kNew),
160-
parent_env_(env), io_thread_req_(),
161-
platform_(platform), dispatching_messages_(false),
162-
session_id_(0), script_name_(path) {
163-
CHECK_EQ(0, uv_async_init(env->event_loop(), &main_thread_req_,
174+
state_(State::kNew), parent_env_(env),
175+
io_thread_req_(), platform_(platform),
176+
dispatching_messages_(false), session_id_(0),
177+
script_name_(path),
178+
wait_for_connect_(wait_for_connect) {
179+
main_thread_req_ = new AsyncAndAgent({uv_async_t(), env->inspector_agent()});
180+
CHECK_EQ(0, uv_async_init(env->event_loop(), &main_thread_req_->first,
164181
InspectorIo::MainThreadAsyncCb));
165-
uv_unref(reinterpret_cast<uv_handle_t*>(&main_thread_req_));
182+
uv_unref(reinterpret_cast<uv_handle_t*>(&main_thread_req_->first));
166183
CHECK_EQ(0, uv_sem_init(&start_sem_, 0));
167184
}
168185

186+
InspectorIo::~InspectorIo() {
187+
uv_sem_destroy(&start_sem_);
188+
uv_close(reinterpret_cast<uv_handle_t*>(&main_thread_req_->first),
189+
ReleasePairOnAsyncClose);
190+
}
191+
169192
bool InspectorIo::Start() {
193+
CHECK_EQ(state_, State::kNew);
170194
CHECK_EQ(uv_thread_create(&thread_, InspectorIo::ThreadCbIO, this), 0);
171195
uv_sem_wait(&start_sem_);
172196

173197
if (state_ == State::kError) {
174-
Stop();
175198
return false;
176199
}
177200
state_ = State::kAccepting;
178-
if (options_.wait_for_connect()) {
201+
if (wait_for_connect_) {
179202
DispatchMessages();
180203
}
181204
return true;
182205
}
183206

184207
void InspectorIo::Stop() {
208+
CHECK(state_ == State::kAccepting || state_ == State::kConnected);
209+
Write(TransportAction::kKill, 0, StringView());
185210
int err = uv_thread_join(&thread_);
186211
CHECK_EQ(err, 0);
212+
state_ = State::kShutDown;
213+
DispatchMessages();
187214
}
188215

189216
bool InspectorIo::IsConnected() {
@@ -195,8 +222,10 @@ bool InspectorIo::IsStarted() {
195222
}
196223

197224
void InspectorIo::WaitForDisconnect() {
225+
if (state_ == State::kAccepting)
226+
state_ = State::kDone;
198227
if (state_ == State::kConnected) {
199-
shutting_down_ = true;
228+
state_ = State::kShutDown;
200229
Write(TransportAction::kStop, 0, StringView());
201230
fprintf(stderr, "Waiting for the debugger to disconnect...\n");
202231
fflush(stderr);
@@ -222,6 +251,9 @@ void InspectorIo::WriteCbIO(uv_async_t* async) {
222251
io->SwapBehindLock(&io->outgoing_message_queue_, &outgoing_messages);
223252
for (const auto& outgoing : outgoing_messages) {
224253
switch (std::get<0>(outgoing)) {
254+
case TransportAction::kKill:
255+
io_and_transport->first->TerminateConnections();
256+
// Fallthrough
225257
case TransportAction::kStop:
226258
io_and_transport->first->Stop(nullptr);
227259
break;
@@ -253,7 +285,7 @@ void InspectorIo::WorkerRunIO() {
253285
uv_fs_req_cleanup(&req);
254286
}
255287
InspectorIoDelegate delegate(this, script_path, script_name_,
256-
options_.wait_for_connect());
288+
wait_for_connect_);
257289
delegate_ = &delegate;
258290
InspectorSocketServer server(&delegate,
259291
options_.host_name(),
@@ -266,14 +298,12 @@ void InspectorIo::WorkerRunIO() {
266298
uv_sem_post(&start_sem_);
267299
return;
268300
}
269-
if (!options_.wait_for_connect()) {
301+
if (!wait_for_connect_) {
270302
uv_sem_post(&start_sem_);
271303
}
272304
uv_run(&loop, UV_RUN_DEFAULT);
273305
io_thread_req_.data = nullptr;
274-
server.Stop(nullptr);
275-
server.TerminateConnections(nullptr);
276-
CHECK_EQ(CloseAsyncAndLoop(&io_thread_req_), 0);
306+
CHECK_EQ(uv_loop_close(&loop), 0);
277307
delegate_ = nullptr;
278308
}
279309

@@ -298,11 +328,12 @@ void InspectorIo::PostIncomingMessage(InspectorAction action, int session_id,
298328
const std::string& message) {
299329
if (AppendMessage(&incoming_message_queue_, action, session_id,
300330
Utf8ToStringView(message))) {
331+
Agent* agent = main_thread_req_->second;
301332
v8::Isolate* isolate = parent_env_->isolate();
302333
platform_->CallOnForegroundThread(isolate,
303-
new DispatchOnInspectorBackendTask(this));
304-
isolate->RequestInterrupt(InterruptCallback, this);
305-
CHECK_EQ(0, uv_async_send(&main_thread_req_));
334+
new DispatchMessagesTask(agent));
335+
isolate->RequestInterrupt(InterruptCallback, agent);
336+
CHECK_EQ(0, uv_async_send(&main_thread_req_->first));
306337
}
307338
NotifyMessageReceived();
308339
}
@@ -344,7 +375,7 @@ void InspectorIo::DispatchMessages() {
344375
break;
345376
case InspectorAction::kEndSession:
346377
CHECK_NE(session_delegate_, nullptr);
347-
if (shutting_down_) {
378+
if (state_ == State::kShutDown) {
348379
state_ = State::kDone;
349380
} else {
350381
state_ = State::kAccepting;
@@ -363,12 +394,18 @@ void InspectorIo::DispatchMessages() {
363394

364395
// static
365396
void InspectorIo::MainThreadAsyncCb(uv_async_t* req) {
366-
InspectorIo* io = node::ContainerOf(&InspectorIo::main_thread_req_, req);
367-
io->DispatchMessages();
397+
AsyncAndAgent* pair = node::ContainerOf(&AsyncAndAgent::first, req);
398+
// Note that this may be called after io was closed or even after a new
399+
// one was created and ran.
400+
InspectorIo* io = pair->second->io();
401+
if (io != nullptr)
402+
io->DispatchMessages();
368403
}
369404

370405
void InspectorIo::Write(TransportAction action, int session_id,
371406
const StringView& inspector_message) {
407+
if (state_ == State::kShutDown)
408+
return;
372409
AppendMessage(&outgoing_message_queue_, action, session_id,
373410
StringBuffer::create(inspector_message));
374411
int err = uv_async_send(&io_thread_req_);

0 commit comments

Comments
 (0)