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_->StopIfWaitingForConnect();
}
#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
21 changes: 21 additions & 0 deletions src/inspector_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,20 @@ class NodeInspectorClient : public V8InspectorClient {
}
}

void StopIfWaitingForFrontendEvent() {
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 +1031,13 @@ void Agent::WaitForConnect() {
client_->waitForFrontend();
}

void Agent::StopIfWaitingForConnect() {
if (client_ == nullptr) {
return;
}
client_->StopIfWaitingForFrontendEvent();
}

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

// Blocks till frontend connects and sends "runIfWaitingForDebugger"
void WaitForConnect();
void StopIfWaitingForConnect();

// Blocks till all the sessions with "WaitForDisconnectOnShutdown" disconnect
void WaitForDisconnect();
void ReportUncaughtException(v8::Local<v8::Value> error,
Expand Down
46 changes: 46 additions & 0 deletions test/parallel/test-inspector-exit-worker-in-wait-for-connection.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
'use strict';

const common = require('../common');
common.skipIfInspectorDisabled();

const { parentPort, workerData, Worker } = require('node:worker_threads');
if (!workerData) {
common.skipIfWorker();
}

const inspector = require('node:inspector');
const assert = require('node:assert');

let TIMEOUT = common.platformTimeout(5000);
if (common.isWindows) {
// Refs: https://github.com/nodejs/build/issues/3014
TIMEOUT = common.platformTimeout(15000);
}

// Refs: https://github.com/nodejs/node/issues/52467

(async () => {
if (!workerData) {
// worker.terminate() should terminate the worker and the pending
// inspector.waitForDebugger().
{
const worker = new Worker(__filename, { workerData: {} });
await new Promise((r) => worker.on('message', r));
await new Promise((r) => setTimeout(r, TIMEOUT));
worker.on('exit', common.mustCall());
await worker.terminate();
}
// process.exit() should kill the process.
{
const worker = new Worker(__filename, { workerData: {} });
await new Promise((r) => worker.on('message', r));
await new Promise((r) => setTimeout(r, TIMEOUT));
process.on('exit', (status) => assert.strictEqual(status, 0));
setImmediate(() => process.exit());
}
} else {
inspector.open(0, undefined, false);
parentPort.postMessage('open');
inspector.waitForDebugger();
}
})().then(common.mustCall());