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.

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

src: fix Worker termination in inspector.waitForDebugger #52527

Merged
merged 9 commits into from
May 13, 2024
7 changes: 7 additions & 0 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1092,6 +1092,13 @@ void Environment::InitializeLibuv() {
void Environment::ExitEnv(StopFlags::Flags flags) {
// Should not access non-thread-safe methods here.
set_stopping(true);

#if HAVE_INSPECTOR
if (inspector_agent_ && inspector_agent_->IsWaitingForConnect()) {
daeyeon marked this conversation as resolved.
Show resolved Hide resolved
inspector_agent_->StopWaitingForConnect();
}
#endif

if ((flags & StopFlags::kDoNotTerminateIsolate) == 0)
isolate_->TerminateExecution();
SetImmediateThreadsafe([](Environment* env) {
Expand Down
11 changes: 10 additions & 1 deletion src/inspector/main_thread_interface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -225,11 +225,20 @@ bool MainThreadInterface::WaitForFrontendEvent() {
dispatching_messages_ = false;
if (dispatching_message_queue_.empty()) {
Mutex::ScopedLock scoped_lock(requests_lock_);
while (requests_.empty()) incoming_message_cond_.Wait(scoped_lock);
while (!stop_waiting_for_frontend_event_requested_ && requests_.empty()) {
incoming_message_cond_.Wait(scoped_lock);
}
stop_waiting_for_frontend_event_requested_ = false;
}
return true;
}

void MainThreadInterface::StopWaitingForFrontendEvent() {
daeyeon marked this conversation as resolved.
Show resolved Hide resolved
Mutex::ScopedLock scoped_lock(requests_lock_);
stop_waiting_for_frontend_event_requested_ = true;
incoming_message_cond_.Broadcast(scoped_lock);
}

void MainThreadInterface::DispatchMessages() {
if (dispatching_messages_)
return;
Expand Down
5 changes: 5 additions & 0 deletions src/inspector/main_thread_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ class MainThreadInterface :
void DispatchMessages();
void Post(std::unique_ptr<Request> request);
bool WaitForFrontendEvent();
void StopWaitingForFrontendEvent();
std::shared_ptr<MainThreadHandle> GetHandle();
Agent* inspector_agent() {
return agent_;
Expand All @@ -94,6 +95,10 @@ class MainThreadInterface :
// when we reenter the DispatchMessages function.
MessageQueue dispatching_message_queue_;
bool dispatching_messages_ = false;
// This flag indicates an internal request to exit the loop in
// WaitForFrontendEvent() It's set to true by calling
// StopWaitingForFrontendEvent().
bool stop_waiting_for_frontend_event_requested_ = false;
daeyeon marked this conversation as resolved.
Show resolved Hide resolved
ConditionVariable incoming_message_cond_;
// Used from any thread
Agent* const agent_;
Expand Down
25 changes: 25 additions & 0 deletions src/inspector_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,22 @@ class NodeInspectorClient : public V8InspectorClient {
}
}

inline bool waiting_for_frontend() { return waiting_for_frontend_; }

void StopWaitingForFrontend() {
if (!waiting_for_frontend_) {
return;
}
waiting_for_frontend_ = false;
daeyeon marked this conversation as resolved.
Show resolved Hide resolved
for (const auto& id_channel : channels_) {
id_channel.second->unsetWaitingForDebugger();
}

if (interface_) {
interface_->StopWaitingForFrontendEvent();
}
}

int connectFrontend(std::unique_ptr<InspectorSessionDelegate> delegate,
bool prevent_shutdown) {
int session_id = next_session_id_++;
Expand Down Expand Up @@ -1017,6 +1033,15 @@ void Agent::WaitForConnect() {
client_->waitForFrontend();
}

void Agent::StopWaitingForConnect() {
CHECK_NOT_NULL(client_);
client_->StopWaitingForFrontend();
}

bool Agent::IsWaitingForConnect() {
return (client_ != nullptr) ? client_->waiting_for_frontend() : false;
}

std::shared_ptr<WorkerManager> Agent::GetWorkerManager() {
THROW_IF_INSUFFICIENT_PERMISSIONS(parent_env_,
permission::PermissionScope::kInspector,
Expand Down
3 changes: 3 additions & 0 deletions src/inspector_agent.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ class Agent {

// Blocks till frontend connects and sends "runIfWaitingForDebugger"
void WaitForConnect();
void StopWaitingForConnect();
bool IsWaitingForConnect();

// Blocks till all the sessions with "WaitForDisconnectOnShutdown" disconnect
void WaitForDisconnect();
void ReportUncaughtException(v8::Local<v8::Value> error,
Expand Down
26 changes: 26 additions & 0 deletions test/parallel/test-inspector-wait-for-connection-on-worker.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Flags: --inspect=0
import * as common from '../common/index.mjs';
common.skipIfInspectorDisabled();

import { isMainThread, Worker } from 'node:worker_threads';
import { fileURLToPath } from 'node:url';
import inspector from 'node:inspector';

// Ref: https://github.com/nodejs/node/issues/52467
if (isMainThread) {
const worker = new Worker(fileURLToPath(import.meta.url));
await new Promise((r) => setTimeout(r, 1_000));

worker.on('exit', common.mustCall());

const timeout = setTimeout(() => {
process.exit();
}, 2_000).unref();

await worker.terminate();

clearTimeout(timeout);
} else {
inspector.open();
inspector.waitForDebugger();
}