Movatterモバイル変換

[0]ホーム
URL:

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

 Sign up
Appearance settings

Commitef5dabd

Browse files
daeyeonmarco-ippolito
authored andcommitted
src: fix Worker termination when '--inspect-brk' is passed
Signed-off-by: Daeyeon Jeong <daeyeon.dev@gmail.com>PR-URL:#53724Fixes:#53648Reviewed-By: James M Snell <jasnell@gmail.com>Reviewed-By: Matteo Collina <matteo.collina@gmail.com>Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
1 parent4647e6b commitef5dabd

File tree

9 files changed

+114
-20
lines changed

9 files changed

+114
-20
lines changed

‎src/debug_utils.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ void NODE_EXTERN_PRIVATE FWrite(FILE* file, const std::string& str);
4545
V(DIAGNOSTICS) \
4646
V(HUGEPAGES) \
4747
V(INSPECTOR_SERVER) \
48+
V(INSPECTOR_CLIENT) \
4849
V(INSPECTOR_PROFILER) \
4950
V(CODE_CACHE) \
5051
V(NGTCP2_DEBUG) \

‎src/env-inl.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -664,6 +664,10 @@ inline bool Environment::should_create_inspector() const {
664664
!options_->test_runner && !options_->watch_mode;
665665
}
666666

667+
inlineboolEnvironment::should_wait_for_inspector_frontend()const {
668+
return (flags_ & EnvironmentFlags::kNoWaitForInspectorFrontend) ==0;
669+
}
670+
667671
inlineboolEnvironment::tracks_unmanaged_fds()const {
668672
return flags_ & EnvironmentFlags::kTrackUnmanagedFds;
669673
}

‎src/env.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -628,6 +628,7 @@ class Environment : public MemoryRetainer {
628628
// the ownership if transferred into the Environment.
629629
voidInitializeInspector(
630630
std::unique_ptr<inspector::ParentInspectorHandle> parent_handle);
631+
voidWaitForInspectorFrontendByOptions();
631632
#endif
632633

633634
inlinesize_tasync_callback_scope_depth()const;
@@ -798,6 +799,7 @@ class Environment : public MemoryRetainer {
798799
inlineboolno_native_addons()const;
799800
inlineboolshould_not_register_esm_loader()const;
800801
inlineboolshould_create_inspector()const;
802+
inlineboolshould_wait_for_inspector_frontend()const;
801803
inlineboolowns_process_state()const;
802804
inlineboolowns_inspector()const;
803805
inlinebooltracks_unmanaged_fds()const;

‎src/inspector_agent.cc

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,8 @@ class NodeInspectorClient : public V8InspectorClient {
487487
}
488488

489489
if (interface_) {
490+
per_process::Debug(DebugCategory::INSPECTOR_CLIENT,
491+
"Stopping waiting for frontend events\n");
490492
interface_->StopWaitingForFrontendEvent();
491493
}
492494
}
@@ -668,11 +670,16 @@ class NodeInspectorClient : public V8InspectorClient {
668670

669671
running_nested_loop_ =true;
670672

673+
per_process::Debug(DebugCategory::INSPECTOR_CLIENT,
674+
"Entering nested loop\n");
675+
671676
while (shouldRunMessageLoop()) {
672677
if (interface_) interface_->WaitForFrontendEvent();
673678
env_->RunAndClearInterrupts();
674679
}
675680
running_nested_loop_ =false;
681+
682+
per_process::Debug(DebugCategory::INSPECTOR_CLIENT,"Exited nested loop\n");
676683
}
677684

678685
doublecurrentTimeMS()override {
@@ -759,27 +766,11 @@ bool Agent::Start(const std::string& path,
759766
}
760767
}, parent_env_);
761768

762-
bool wait_for_connect = options.wait_for_connect();
763-
bool should_break_first_line = options.should_break_first_line();
764-
if (parent_handle_) {
765-
should_break_first_line = parent_handle_->WaitForConnect();
766-
parent_handle_->WorkerStarted(client_->getThreadHandle(),
767-
should_break_first_line);
768-
}elseif (!options.inspector_enabled || !options.allow_attaching_debugger ||
769-
!StartIoThread()) {
769+
if (!parent_handle_ &&
770+
(!options.inspector_enabled || !options.allow_attaching_debugger ||
771+
!StartIoThread())) {
770772
returnfalse;
771773
}
772-
773-
if (wait_for_connect || should_break_first_line) {
774-
// Patch the debug options to implement waitForDebuggerOnStart for
775-
// the NodeWorker.enable method.
776-
if (should_break_first_line) {
777-
CHECK(!parent_env_->has_serialized_options());
778-
debug_options_.EnableBreakFirstLine();
779-
parent_env_->options()->get_debug_options()->EnableBreakFirstLine();
780-
}
781-
client_->waitForFrontend();
782-
}
783774
returntrue;
784775
}
785776

@@ -1038,6 +1029,33 @@ void Agent::WaitForConnect() {
10381029
client_->waitForFrontend();
10391030
}
10401031

1032+
boolAgent::WaitForConnectByOptions() {
1033+
if (client_ ==nullptr) {
1034+
returnfalse;
1035+
}
1036+
1037+
bool wait_for_connect = debug_options_.wait_for_connect();
1038+
bool should_break_first_line = debug_options_.should_break_first_line();
1039+
if (parent_handle_) {
1040+
should_break_first_line = parent_handle_->WaitForConnect();
1041+
parent_handle_->WorkerStarted(client_->getThreadHandle(),
1042+
should_break_first_line);
1043+
}
1044+
1045+
if (wait_for_connect || should_break_first_line) {
1046+
// Patch the debug options to implement waitForDebuggerOnStart for
1047+
// the NodeWorker.enable method.
1048+
if (should_break_first_line) {
1049+
CHECK(!parent_env_->has_serialized_options());
1050+
debug_options_.EnableBreakFirstLine();
1051+
parent_env_->options()->get_debug_options()->EnableBreakFirstLine();
1052+
}
1053+
client_->waitForFrontend();
1054+
returntrue;
1055+
}
1056+
returnfalse;
1057+
}
1058+
10411059
voidAgent::StopIfWaitingForConnect() {
10421060
if (client_ ==nullptr) {
10431061
return;

‎src/inspector_agent.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ class Agent {
6161

6262
// Blocks till frontend connects and sends "runIfWaitingForDebugger"
6363
voidWaitForConnect();
64+
boolWaitForConnectByOptions();
6465
voidStopIfWaitingForConnect();
6566

6667
// Blocks till all the sessions with "WaitForDisconnectOnShutdown" disconnect

‎src/node.cc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,17 @@ void Environment::InitializeInspector(
204204
return;
205205
}
206206

207+
if (should_wait_for_inspector_frontend()) {
208+
WaitForInspectorFrontendByOptions();
209+
}
210+
207211
profiler::StartProfilers(this);
212+
}
213+
214+
voidEnvironment::WaitForInspectorFrontendByOptions() {
215+
if (!inspector_agent_->WaitForConnectByOptions()) {
216+
return;
217+
}
208218

209219
if (inspector_agent_->options().break_node_first_line) {
210220
inspector_agent_->PauseOnNextJavascriptStatement("Break at bootstrap");

‎src/node.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,11 @@ enum Flags : uint64_t {
661661
// Controls where or not the InspectorAgent for this Environment should
662662
// call StartDebugSignalHandler. This control is needed by embedders who may
663663
// not want to allow other processes to start the V8 inspector.
664-
kNoStartDebugSignalHandler =1 <<10
664+
kNoStartDebugSignalHandler =1 <<10,
665+
// Controls whether the InspectorAgent created for this Environment waits for
666+
// Inspector frontend events during the Environment creation. It's used to
667+
// call node::Stop(env) on a Worker thread that is waiting for the events.
668+
kNoWaitForInspectorFrontend =1 <<11
665669
};
666670
}// namespace EnvironmentFlags
667671

‎src/node_worker.cc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,9 @@ void Worker::Run() {
359359
CHECK(!context.IsEmpty());
360360
Context::Scopecontext_scope(context);
361361
{
362+
#if HAVE_INSPECTOR
363+
environment_flags_ |= EnvironmentFlags::kNoWaitForInspectorFrontend;
364+
#endif
362365
env_.reset(CreateEnvironment(
363366
data.isolate_data_.get(),
364367
context,
@@ -380,6 +383,10 @@ void Worker::Run() {
380383
this->env_ = env_.get();
381384
}
382385
Debug(this,"Created Environment for worker with id %llu", thread_id_.id);
386+
387+
#if HAVE_INSPECTOR
388+
this->env_->WaitForInspectorFrontendByOptions();
389+
#endif
383390
if (is_stopped())return;
384391
{
385392
if (!CreateEnvMessagePort(env_.get())) {
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
'use strict';
2+
3+
constcommon=require('../common');
4+
common.skipIfInspectorDisabled();
5+
6+
const{ workerData, Worker}=require('node:worker_threads');
7+
if(!workerData){
8+
common.skipIfWorker();
9+
}
10+
11+
constassert=require('node:assert');
12+
13+
letTIMEOUT=common.platformTimeout(5000);
14+
if(common.isWindows){
15+
// Refs: https://github.com/nodejs/build/issues/3014
16+
TIMEOUT=common.platformTimeout(15000);
17+
}
18+
19+
// Refs: https://github.com/nodejs/node/issues/53648
20+
21+
(async()=>{
22+
if(!workerData){
23+
// worker.terminate() should terminate the worker created with execArgv:
24+
// ["--inspect-brk"].
25+
{
26+
constworker=newWorker(__filename,{
27+
execArgv:['--inspect-brk=0'],
28+
workerData:{},
29+
});
30+
awaitnewPromise((r)=>setTimeout(r,TIMEOUT));
31+
worker.on('exit',common.mustCall());
32+
awaitworker.terminate();
33+
}
34+
// process.exit() should kill the process.
35+
{
36+
newWorker(__filename,{
37+
execArgv:['--inspect-brk=0'],
38+
workerData:{},
39+
});
40+
awaitnewPromise((r)=>setTimeout(r,TIMEOUT));
41+
process.on('exit',(status)=>assert.strictEqual(status,0));
42+
setImmediate(()=>process.exit());
43+
}
44+
}else{
45+
console.log('Worker running!');
46+
}
47+
})().then(common.mustCall());

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp