FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

inspector: Fix crash for WS connection · nodejs/node@54cd7df · GitHub

/ node Public

Commit 54cd7df

Browse files
Eugene Ostroukhov
authored and
MylesBorins
committed
inspector: Fix crash for WS connection
Attaching WS session will now include a roundtrip onto the main thread to make sure there is no other session (e.g. JS bindings) This change also required refactoring WS socket implementation to better support scenarios like this. Fixes: #16852 PR-URL: #17085 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com>
1 parent aa32bd0 commit 54cd7df

13 files changed

Lines changed: 1022 additions & 1024 deletions

‎src/inspector_agent.cc‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -548,10 +548,6 @@ void Agent::Connect(InspectorSessionDelegate* delegate) {
548548
client_->connectFrontend(delegate);
549549
}
550550

551-
bool Agent::IsConnected() {
552-
return io_ && io_->IsConnected();
553-
}
554-
555551
void Agent::WaitForDisconnect() {
556552
CHECK_NE(client_, nullptr);
557553
client_->contextDestroyed(parent_env_->context());

‎src/inspector_agent.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ class Agent {
4848
bool IsStarted() { return !!client_; }
4949

5050
// IO thread started, and client connected
51-
bool IsConnected();
5251
bool IsWaitingForConnect();
5352

5453
void WaitForDisconnect();

‎src/inspector_io.cc‎

Lines changed: 47 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ class InspectorIoDelegate: public node::inspector::SocketServerDelegate {
136136
const std::string& script_name, bool wait);
137137
// Calls PostIncomingMessage() with appropriate InspectorAction:
138138
// kStartSession
139-
bool StartSession(int session_id, const std::string& target_id) override;
139+
void StartSession(int session_id, const std::string& target_id) override;
140140
// kSendMessage
141141
void MessageReceived(int session_id, const std::string& message) override;
142142
// kEndSession
@@ -145,19 +145,22 @@ class InspectorIoDelegate: public node::inspector::SocketServerDelegate {
145145
std::vector<std::string> GetTargetIds() override;
146146
std::string GetTargetTitle(const std::string& id) override;
147147
std::string GetTargetUrl(const std::string& id) override;
148-
bool IsConnected() { return connected_; }
149148
void ServerDone() override {
150149
io_->ServerDone();
151150
}
152151

152+
void AssignTransport(InspectorSocketServer* server) {
153+
server_ = server;
154+
}
155+
153156
private:
154157
InspectorIo* io_;
155-
bool connected_;
156158
int session_id_;
157159
const std::string script_name_;
158160
const std::string script_path_;
159161
const std::string target_id_;
160162
bool waiting_;
163+
InspectorSocketServer* server_;
161164
};
162165

163166
void InterruptCallback(v8::Isolate*, void* agent) {
@@ -226,10 +229,6 @@ void InspectorIo::Stop() {
226229
DispatchMessages();
227230
}
228231

229-
bool InspectorIo::IsConnected() {
230-
return delegate_ != nullptr && delegate_->IsConnected();
231-
}
232-
233232
bool InspectorIo::IsStarted() {
234233
return platform_ != nullptr;
235234
}
@@ -264,6 +263,7 @@ void InspectorIo::IoThreadAsyncCb(uv_async_t* async) {
264263
MessageQueue<TransportAction> outgoing_message_queue;
265264
io->SwapBehindLock(&io->outgoing_message_queue_, &outgoing_message_queue);
266265
for (const auto& outgoing : outgoing_message_queue) {
266+
int session_id = std::get<1>(outgoing);
267267
switch (std::get<0>(outgoing)) {
268268
case TransportAction::kKill:
269269
transport->TerminateConnections();
@@ -272,8 +272,14 @@ void InspectorIo::IoThreadAsyncCb(uv_async_t* async) {
272272
transport->Stop(nullptr);
273273
break;
274274
case TransportAction::kSendMessage:
275-
std::string message = StringViewToUtf8(std::get<2>(outgoing)->string());
276-
transport->Send(std::get<1>(outgoing), message);
275+
transport->Send(session_id,
276+
StringViewToUtf8(std::get<2>(outgoing)->string()));
277+
break;
278+
case TransportAction::kAcceptSession:
279+
transport->AcceptSession(session_id);
280+
break;
281+
case TransportAction::kDeclineSession:
282+
transport->DeclineSession(session_id);
277283
break;
278284
}
279285
}
@@ -293,6 +299,7 @@ void InspectorIo::ThreadMain() {
293299
wait_for_connect_);
294300
delegate_ = &delegate;
295301
Transport server(&delegate, &loop, options_.host_name(), options_.port());
302+
delegate.AssignTransport(&server);
296303
TransportAndIo<Transport> queue_transport(&server, this);
297304
thread_req_.data = &queue_transport;
298305
if (!server.Start()) {
@@ -308,6 +315,7 @@ void InspectorIo::ThreadMain() {
308315
uv_run(&loop, UV_RUN_DEFAULT);
309316
thread_req_.data = nullptr;
310317
CHECK_EQ(uv_loop_close(&loop), 0);
318+
delegate.AssignTransport(nullptr);
311319
delegate_ = nullptr;
312320
}
313321

@@ -358,6 +366,21 @@ void InspectorIo::NotifyMessageReceived() {
358366
incoming_message_cond_.Broadcast(scoped_lock);
359367
}
360368

369+
TransportAction InspectorIo::Attach(int session_id) {
370+
Agent* agent = parent_env_->inspector_agent();
371+
if (agent->delegate() != nullptr)
372+
return TransportAction::kDeclineSession;
373+
374+
CHECK_EQ(session_delegate_, nullptr);
375+
session_id_ = session_id;
376+
state_ = State::kConnected;
377+
fprintf(stderr, "Debugger attached.\n");
378+
session_delegate_ = std::unique_ptr<InspectorSessionDelegate>(
379+
new IoSessionDelegate(this));
380+
agent->Connect(session_delegate_.get());
381+
return TransportAction::kAcceptSession;
382+
}
383+
361384
void InspectorIo::DispatchMessages() {
362385
// This function can be reentered if there was an incoming message while
363386
// V8 was processing another inspector request (e.g. if the user is
@@ -375,16 +398,14 @@ void InspectorIo::DispatchMessages() {
375398
MessageQueue<InspectorAction>::value_type task;
376399
std::swap(dispatching_message_queue_.front(), task);
377400
dispatching_message_queue_.pop_front();
401+
int id = std::get<1>(task);
378402
StringView message = std::get<2>(task)->string();
379403
switch (std::get<0>(task)) {
380404
case InspectorAction::kStartSession:
381-
CHECK_EQ(session_delegate_, nullptr);
382-
session_id_ = std::get<1>(task);
383-
state_ = State::kConnected;
384-
fprintf(stderr, "Debugger attached.\n");
385-
session_delegate_ = std::unique_ptr<InspectorSessionDelegate>(
386-
new IoSessionDelegate(this));
387-
parent_env_->inspector_agent()->Connect(session_delegate_.get());
405+
Write(Attach(id), id, StringView());
406+
break;
407+
case InspectorAction::kStartSessionUnconditionally:
408+
Attach(id);
388409
break;
389410
case InspectorAction::kEndSession:
390411
CHECK_NE(session_delegate_, nullptr);
@@ -428,22 +449,23 @@ InspectorIoDelegate::InspectorIoDelegate(InspectorIo* io,
428449
const std::string& script_name,
429450
bool wait)
430451
: io_(io),
431-
connected_(false),
432452
session_id_(0),
433453
script_name_(script_name),
434454
script_path_(script_path),
435455
target_id_(GenerateID()),
436-
waiting_(wait) { }
456+
waiting_(wait),
457+
server_(nullptr) { }
437458

438459

439-
bool InspectorIoDelegate::StartSession(int session_id,
460+
void InspectorIoDelegate::StartSession(int session_id,
440461
const std::string& target_id) {
441-
if (connected_)
442-
return false;
443-
connected_ = true;
444-
session_id_++;
445-
io_->PostIncomingMessage(InspectorAction::kStartSession, session_id, "");
446-
return true;
462+
session_id_ = session_id;
463+
InspectorAction action = InspectorAction::kStartSession;
464+
if (waiting_) {
465+
action = InspectorAction::kStartSessionUnconditionally;
466+
server_->AcceptSession(session_id);
467+
}
468+
io_->PostIncomingMessage(action, session_id, "");
447469
}
448470

449471
void InspectorIoDelegate::MessageReceived(int session_id,
@@ -464,7 +486,6 @@ void InspectorIoDelegate::MessageReceived(int session_id,
464486
}
465487

466488
void InspectorIoDelegate::EndSession(int session_id) {
467-
connected_ = false;
468489
io_->PostIncomingMessage(InspectorAction::kEndSession, session_id, "");
469490
}
470491

‎src/inspector_io.h‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ class InspectorIoDelegate;
3636

3737
enum class InspectorAction {
3838
kStartSession,
39+
kStartSessionUnconditionally, // First attach with --inspect-brk
3940
kEndSession,
4041
kSendMessage
4142
};
@@ -44,7 +45,9 @@ enum class InspectorAction {
4445
enum class TransportAction {
4546
kKill,
4647
kSendMessage,
47-
kStop
48+
kStop,
49+
kAcceptSession,
50+
kDeclineSession
4851
};
4952

5053
class InspectorIo {
@@ -61,7 +64,6 @@ class InspectorIo {
6164
void Stop();
6265

6366
bool IsStarted();
64-
bool IsConnected();
6567

6668
void WaitForDisconnect();
6769
// Called from thread to queue an incoming message and trigger
@@ -124,6 +126,8 @@ class InspectorIo {
124126
void WaitForFrontendMessageWhilePaused();
125127
// Broadcast incoming_message_cond_
126128
void NotifyMessageReceived();
129+
// Attach session to an inspector. Either kAcceptSession or kDeclineSession
130+
TransportAction Attach(int session_id);
127131

128132
const DebugOptions options_;
129133

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL