Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 663bb97

Browse filesBrowse files
daeyeontargos
authored andcommitted
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 ef6a210 commit 663bb97
Copy full SHA for 663bb97

File tree

Expand file treeCollapse file tree

6 files changed

+91
-1
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

6 files changed

+91
-1
lines changed
Open diff view settings
Collapse file

‎src/env.cc‎

Copy file name to clipboardExpand all lines: src/env.cc
+7Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1110,6 +1110,13 @@ void Environment::InitializeCompileCache() {
11101110
void Environment::ExitEnv(StopFlags::Flags flags) {
11111111
// Should not access non-thread-safe methods here.
11121112
set_stopping(true);
1113+
1114+
#if HAVE_INSPECTOR
1115+
if (inspector_agent_) {
1116+
inspector_agent_->StopIfWaitingForConnect();
1117+
}
1118+
#endif
1119+
11131120
if ((flags & StopFlags::kDoNotTerminateIsolate) == 0)
11141121
isolate_->TerminateExecution();
11151122
SetImmediateThreadsafe([](Environment* env) {
Collapse file

‎src/inspector/main_thread_interface.cc‎

Copy file name to clipboardExpand all lines: src/inspector/main_thread_interface.cc
+10-1Lines changed: 10 additions & 1 deletion
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;
Collapse file

‎src/inspector/main_thread_interface.h‎

Copy file name to clipboardExpand all lines: src/inspector/main_thread_interface.h
+5Lines changed: 5 additions & 0 deletions
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_;
Collapse file

‎src/inspector_agent.cc‎

Copy file name to clipboardExpand all lines: src/inspector_agent.cc
+21Lines changed: 21 additions & 0 deletions
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,
Collapse file

‎src/inspector_agent.h‎

Copy file name to clipboardExpand all lines: src/inspector_agent.h
+2Lines changed: 2 additions & 0 deletions
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,
Collapse file
+46Lines changed: 46 additions & 0 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.