Skip to content

Commit 89a910b

Browse files
daeyeonmarco-ippolito
authored andcommittedJun 17, 2024
src: fix Worker termination in inspector.waitForDebugger
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>
1 parent be9d6f2 commit 89a910b

6 files changed

+91
-1
lines changed
 

‎src/env.cc

+7
Original file line numberDiff line numberDiff line change
@@ -1090,6 +1090,13 @@ void Environment::InitializeLibuv() {
10901090
void Environment::ExitEnv(StopFlags::Flags flags) {
10911091
// Should not access non-thread-safe methods here.
10921092
set_stopping(true);
1093+
1094+
#if HAVE_INSPECTOR
1095+
if (inspector_agent_) {
1096+
inspector_agent_->StopIfWaitingForConnect();
1097+
}
1098+
#endif
1099+
10931100
if ((flags & StopFlags::kDoNotTerminateIsolate) == 0)
10941101
isolate_->TerminateExecution();
10951102
SetImmediateThreadsafe([](Environment* env) {

‎src/inspector/main_thread_interface.cc

+10-1
Original file line numberDiff line numberDiff line change
@@ -225,11 +225,20 @@ bool MainThreadInterface::WaitForFrontendEvent() {
225225
dispatching_messages_ = false;
226226
if (dispatching_message_queue_.empty()) {
227227
Mutex::ScopedLock scoped_lock(requests_lock_);
228-
while (requests_.empty()) incoming_message_cond_.Wait(scoped_lock);
228+
while (!stop_waiting_for_frontend_event_requested_ && requests_.empty()) {
229+
incoming_message_cond_.Wait(scoped_lock);
230+
}
231+
stop_waiting_for_frontend_event_requested_ = false;
229232
}
230233
return true;
231234
}
232235

236+
void MainThreadInterface::StopWaitingForFrontendEvent() {
237+
Mutex::ScopedLock scoped_lock(requests_lock_);
238+
stop_waiting_for_frontend_event_requested_ = true;
239+
incoming_message_cond_.Broadcast(scoped_lock);
240+
}
241+
233242
void MainThreadInterface::DispatchMessages() {
234243
if (dispatching_messages_)
235244
return;

‎src/inspector/main_thread_interface.h

+5
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ class MainThreadInterface :
7878
void DispatchMessages();
7979
void Post(std::unique_ptr<Request> request);
8080
bool WaitForFrontendEvent();
81+
void StopWaitingForFrontendEvent();
8182
std::shared_ptr<MainThreadHandle> GetHandle();
8283
Agent* inspector_agent() {
8384
return agent_;
@@ -94,6 +95,10 @@ class MainThreadInterface :
9495
// when we reenter the DispatchMessages function.
9596
MessageQueue dispatching_message_queue_;
9697
bool dispatching_messages_ = false;
98+
// This flag indicates an internal request to exit the loop in
99+
// WaitForFrontendEvent(). It's set to true by calling
100+
// StopWaitingForFrontendEvent().
101+
bool stop_waiting_for_frontend_event_requested_ = false;
97102
ConditionVariable incoming_message_cond_;
98103
// Used from any thread
99104
Agent* const agent_;

‎src/inspector_agent.cc

+21
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,20 @@ class NodeInspectorClient : public V8InspectorClient {
477477
}
478478
}
479479

480+
void StopIfWaitingForFrontendEvent() {
481+
if (!waiting_for_frontend_) {
482+
return;
483+
}
484+
waiting_for_frontend_ = false;
485+
for (const auto& id_channel : channels_) {
486+
id_channel.second->unsetWaitingForDebugger();
487+
}
488+
489+
if (interface_) {
490+
interface_->StopWaitingForFrontendEvent();
491+
}
492+
}
493+
480494
int connectFrontend(std::unique_ptr<InspectorSessionDelegate> delegate,
481495
bool prevent_shutdown) {
482496
int session_id = next_session_id_++;
@@ -1024,6 +1038,13 @@ void Agent::WaitForConnect() {
10241038
client_->waitForFrontend();
10251039
}
10261040

1041+
void Agent::StopIfWaitingForConnect() {
1042+
if (client_ == nullptr) {
1043+
return;
1044+
}
1045+
client_->StopIfWaitingForFrontendEvent();
1046+
}
1047+
10271048
std::shared_ptr<WorkerManager> Agent::GetWorkerManager() {
10281049
THROW_IF_INSUFFICIENT_PERMISSIONS(parent_env_,
10291050
permission::PermissionScope::kInspector,

‎src/inspector_agent.h

+2
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ class Agent {
6161

6262
// Blocks till frontend connects and sends "runIfWaitingForDebugger"
6363
void WaitForConnect();
64+
void StopIfWaitingForConnect();
65+
6466
// Blocks till all the sessions with "WaitForDisconnectOnShutdown" disconnect
6567
void WaitForDisconnect();
6668
void ReportUncaughtException(v8::Local<v8::Value> error,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
common.skipIfInspectorDisabled();
5+
6+
const { parentPort, workerData, Worker } = require('node:worker_threads');
7+
if (!workerData) {
8+
common.skipIfWorker();
9+
}
10+
11+
const inspector = require('node:inspector');
12+
const assert = require('node:assert');
13+
14+
let TIMEOUT = common.platformTimeout(5000);
15+
if (common.isWindows) {
16+
// Refs: https://github.com/nodejs/build/issues/3014
17+
TIMEOUT = common.platformTimeout(15000);
18+
}
19+
20+
// Refs: https://github.com/nodejs/node/issues/52467
21+
22+
(async () => {
23+
if (!workerData) {
24+
// worker.terminate() should terminate the worker and the pending
25+
// inspector.waitForDebugger().
26+
{
27+
const worker = new Worker(__filename, { workerData: {} });
28+
await new Promise((r) => worker.on('message', r));
29+
await new Promise((r) => setTimeout(r, TIMEOUT));
30+
worker.on('exit', common.mustCall());
31+
await worker.terminate();
32+
}
33+
// process.exit() should kill the process.
34+
{
35+
const worker = new Worker(__filename, { workerData: {} });
36+
await new Promise((r) => worker.on('message', r));
37+
await new Promise((r) => setTimeout(r, TIMEOUT));
38+
process.on('exit', (status) => assert.strictEqual(status, 0));
39+
setImmediate(() => process.exit());
40+
}
41+
} else {
42+
inspector.open(0, undefined, false);
43+
parentPort.postMessage('open');
44+
inspector.waitForDebugger();
45+
}
46+
})().then(common.mustCall());

0 commit comments

Comments
 (0)
Please sign in to comment.