diff --git a/src/debug_utils.h b/src/debug_utils.h index 5fbc0010f8ff8a..d00e2d1ca9ac4b 100644 --- a/src/debug_utils.h +++ b/src/debug_utils.h @@ -45,6 +45,7 @@ void NODE_EXTERN_PRIVATE FWrite(FILE* file, const std::string& str); V(DIAGNOSTICS) \ V(HUGEPAGES) \ V(INSPECTOR_SERVER) \ + V(INSPECTOR_CLIENT) \ V(INSPECTOR_PROFILER) \ V(CODE_CACHE) \ V(NGTCP2_DEBUG) \ diff --git a/src/env-inl.h b/src/env-inl.h index 63ce35ba68b48a..18b1461e50e456 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -664,6 +664,10 @@ inline bool Environment::should_create_inspector() const { !options_->test_runner && !options_->watch_mode; } +inline bool Environment::should_wait_for_inspector_frontend() const { + return (flags_ & EnvironmentFlags::kNoWaitForInspectorFrontend) == 0; +} + inline bool Environment::tracks_unmanaged_fds() const { return flags_ & EnvironmentFlags::kTrackUnmanagedFds; } diff --git a/src/env.h b/src/env.h index cd8db07919dc4d..1ca2d5ed40fa3c 100644 --- a/src/env.h +++ b/src/env.h @@ -628,6 +628,7 @@ class Environment : public MemoryRetainer { // the ownership if transferred into the Environment. void InitializeInspector( std::unique_ptr parent_handle); + void WaitForInspectorFrontendByOptions(); #endif inline size_t async_callback_scope_depth() const; @@ -798,6 +799,7 @@ class Environment : public MemoryRetainer { inline bool no_native_addons() const; inline bool should_not_register_esm_loader() const; inline bool should_create_inspector() const; + inline bool should_wait_for_inspector_frontend() const; inline bool owns_process_state() const; inline bool owns_inspector() const; inline bool tracks_unmanaged_fds() const; diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index f298ab73285f4e..4cab7dea04379c 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -487,6 +487,8 @@ class NodeInspectorClient : public V8InspectorClient { } if (interface_) { + per_process::Debug(DebugCategory::INSPECTOR_CLIENT, + "Stopping waiting for frontend events\n"); interface_->StopWaitingForFrontendEvent(); } } @@ -668,11 +670,16 @@ class NodeInspectorClient : public V8InspectorClient { running_nested_loop_ = true; + per_process::Debug(DebugCategory::INSPECTOR_CLIENT, + "Entering nested loop\n"); + while (shouldRunMessageLoop()) { if (interface_) interface_->WaitForFrontendEvent(); env_->RunAndClearInterrupts(); } running_nested_loop_ = false; + + per_process::Debug(DebugCategory::INSPECTOR_CLIENT, "Exited nested loop\n"); } double currentTimeMS() override { @@ -759,27 +766,11 @@ bool Agent::Start(const std::string& path, } }, parent_env_); - bool wait_for_connect = options.wait_for_connect(); - bool should_break_first_line = options.should_break_first_line(); - if (parent_handle_) { - should_break_first_line = parent_handle_->WaitForConnect(); - parent_handle_->WorkerStarted(client_->getThreadHandle(), - should_break_first_line); - } else if (!options.inspector_enabled || !options.allow_attaching_debugger || - !StartIoThread()) { + if (!parent_handle_ && + (!options.inspector_enabled || !options.allow_attaching_debugger || + !StartIoThread())) { return false; } - - if (wait_for_connect || should_break_first_line) { - // Patch the debug options to implement waitForDebuggerOnStart for - // the NodeWorker.enable method. - if (should_break_first_line) { - CHECK(!parent_env_->has_serialized_options()); - debug_options_.EnableBreakFirstLine(); - parent_env_->options()->get_debug_options()->EnableBreakFirstLine(); - } - client_->waitForFrontend(); - } return true; } @@ -1038,6 +1029,33 @@ void Agent::WaitForConnect() { client_->waitForFrontend(); } +bool Agent::WaitForConnectByOptions() { + if (client_ == nullptr) { + return false; + } + + bool wait_for_connect = debug_options_.wait_for_connect(); + bool should_break_first_line = debug_options_.should_break_first_line(); + if (parent_handle_) { + should_break_first_line = parent_handle_->WaitForConnect(); + parent_handle_->WorkerStarted(client_->getThreadHandle(), + should_break_first_line); + } + + if (wait_for_connect || should_break_first_line) { + // Patch the debug options to implement waitForDebuggerOnStart for + // the NodeWorker.enable method. + if (should_break_first_line) { + CHECK(!parent_env_->has_serialized_options()); + debug_options_.EnableBreakFirstLine(); + parent_env_->options()->get_debug_options()->EnableBreakFirstLine(); + } + client_->waitForFrontend(); + return true; + } + return false; +} + void Agent::StopIfWaitingForConnect() { if (client_ == nullptr) { return; diff --git a/src/inspector_agent.h b/src/inspector_agent.h index 0f27aff61a3955..725275e43c7135 100644 --- a/src/inspector_agent.h +++ b/src/inspector_agent.h @@ -61,6 +61,7 @@ class Agent { // Blocks till frontend connects and sends "runIfWaitingForDebugger" void WaitForConnect(); + bool WaitForConnectByOptions(); void StopIfWaitingForConnect(); // Blocks till all the sessions with "WaitForDisconnectOnShutdown" disconnect diff --git a/src/node.cc b/src/node.cc index 95d2ffa49370f3..9f6f8e53abd7e4 100644 --- a/src/node.cc +++ b/src/node.cc @@ -204,7 +204,17 @@ void Environment::InitializeInspector( return; } + if (should_wait_for_inspector_frontend()) { + WaitForInspectorFrontendByOptions(); + } + profiler::StartProfilers(this); +} + +void Environment::WaitForInspectorFrontendByOptions() { + if (!inspector_agent_->WaitForConnectByOptions()) { + return; + } if (inspector_agent_->options().break_node_first_line) { inspector_agent_->PauseOnNextJavascriptStatement("Break at bootstrap"); diff --git a/src/node.h b/src/node.h index 7047a667f7f1b2..7005c2b4449986 100644 --- a/src/node.h +++ b/src/node.h @@ -661,7 +661,11 @@ enum Flags : uint64_t { // Controls where or not the InspectorAgent for this Environment should // call StartDebugSignalHandler. This control is needed by embedders who may // not want to allow other processes to start the V8 inspector. - kNoStartDebugSignalHandler = 1 << 10 + kNoStartDebugSignalHandler = 1 << 10, + // Controls whether the InspectorAgent created for this Environment waits for + // Inspector frontend events during the Environment creation. It's used to + // call node::Stop(env) on a Worker thread that is waiting for the events. + kNoWaitForInspectorFrontend = 1 << 11 }; } // namespace EnvironmentFlags diff --git a/src/node_worker.cc b/src/node_worker.cc index f12dde0ae50a6c..baa070145235a0 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -359,6 +359,9 @@ void Worker::Run() { CHECK(!context.IsEmpty()); Context::Scope context_scope(context); { +#if HAVE_INSPECTOR + environment_flags_ |= EnvironmentFlags::kNoWaitForInspectorFrontend; +#endif env_.reset(CreateEnvironment( data.isolate_data_.get(), context, @@ -380,6 +383,10 @@ void Worker::Run() { this->env_ = env_.get(); } Debug(this, "Created Environment for worker with id %llu", thread_id_.id); + +#if HAVE_INSPECTOR + this->env_->WaitForInspectorFrontendByOptions(); +#endif if (is_stopped()) return; { if (!CreateEnvMessagePort(env_.get())) { diff --git a/test/parallel/test-inspector-exit-worker-in-wait-for-connection2.js b/test/parallel/test-inspector-exit-worker-in-wait-for-connection2.js new file mode 100644 index 00000000000000..fb13fc3f969304 --- /dev/null +++ b/test/parallel/test-inspector-exit-worker-in-wait-for-connection2.js @@ -0,0 +1,47 @@ +'use strict'; + +const common = require('../common'); +common.skipIfInspectorDisabled(); + +const { workerData, Worker } = require('node:worker_threads'); +if (!workerData) { + common.skipIfWorker(); +} + +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/53648 + +(async () => { + if (!workerData) { + // worker.terminate() should terminate the worker created with execArgv: + // ["--inspect-brk"]. + { + const worker = new Worker(__filename, { + execArgv: ['--inspect-brk=0'], + workerData: {}, + }); + await new Promise((r) => setTimeout(r, TIMEOUT)); + worker.on('exit', common.mustCall()); + await worker.terminate(); + } + // process.exit() should kill the process. + { + new Worker(__filename, { + execArgv: ['--inspect-brk=0'], + workerData: {}, + }); + await new Promise((r) => setTimeout(r, TIMEOUT)); + process.on('exit', (status) => assert.strictEqual(status, 0)); + setImmediate(() => process.exit()); + } + } else { + console.log('Worker running!'); + } +})().then(common.mustCall());