Skip to content

Commit

Permalink
src: fix Worker termination in inspector.waitForDebugger
Browse files Browse the repository at this point in the history
Signed-off-by: Daeyeon Jeong <daeyeon.dev@gmail.com>
PR-URL: #52527
Fixes: #52467
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
  • Loading branch information
daeyeon authored and targos committed May 13, 2024
1 parent ef6a210 commit 663bb97
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 1 deletion.
7 changes: 7 additions & 0 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1110,6 +1110,13 @@ void Environment::InitializeCompileCache() {
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() {
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;
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;
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 @@ -1024,6 +1038,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());

0 comments on commit 663bb97

Please sign in to comment.