From c811b8a1cbe2be0d35f9e027e0385ad99f4e582a Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Thu, 24 Feb 2022 00:32:30 +0100 Subject: [PATCH] src: allow preventing InitializeInspector in env PR-URL: https://github.com/nodejs/node/pull/35025 Reviewed-By: David Carlier Reviewed-By: Anna Henningsen Reviewed-By: Joyee Cheung Reviewed-By: Rich Trott Reviewed-By: James M Snell --- src/api/environment.cc | 14 +++++++------ src/env-inl.h | 4 ++++ src/env.h | 1 + src/inspector_agent.cc | 47 ++++++++++++++++++++++++++++++++++++++++++ src/node.h | 7 ++++++- 5 files changed, 66 insertions(+), 7 deletions(-) diff --git a/src/api/environment.cc b/src/api/environment.cc index 0fb750c5abbe00..55b895c235f51e 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -344,12 +344,14 @@ Environment* CreateEnvironment( Environment* env = new Environment( isolate_data, context, args, exec_args, nullptr, flags, thread_id); #if HAVE_INSPECTOR - if (inspector_parent_handle) { - env->InitializeInspector( - std::move(static_cast( - inspector_parent_handle.get())->impl)); - } else { - env->InitializeInspector({}); + if (env->should_create_inspector()) { + if (inspector_parent_handle) { + env->InitializeInspector( + std::move(static_cast( + inspector_parent_handle.get())->impl)); + } else { + env->InitializeInspector({}); + } } #endif diff --git a/src/env-inl.h b/src/env-inl.h index e679780900abc9..4a34393cad7e07 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -869,6 +869,10 @@ inline bool Environment::owns_inspector() const { return flags_ & EnvironmentFlags::kOwnsInspector; } +inline bool Environment::should_create_inspector() const { + return (flags_ & EnvironmentFlags::kNoCreateInspector) == 0; +} + inline bool Environment::tracks_unmanaged_fds() const { return flags_ & EnvironmentFlags::kTrackUnmanagedFds; } diff --git a/src/env.h b/src/env.h index 7aa5822abf11f1..cda7a52fa1ffc6 100644 --- a/src/env.h +++ b/src/env.h @@ -1210,6 +1210,7 @@ class Environment : public MemoryRetainer { inline bool is_main_thread() const; inline bool no_native_addons() const; inline bool should_not_register_esm_loader() const; + inline bool should_create_inspector() 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 fd9f514b9b6a7b..5fc533741d7c8d 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -368,6 +368,16 @@ bool IsFilePath(const std::string& path) { } #endif // __POSIX__ +void ThrowUninitializedInspectorError(Environment* env) { + HandleScope scope(env->isolate()); + + const char* msg = "This Environment was initialized without a V8::Inspector"; + Local exception = + v8::String::NewFromUtf8(env->isolate(), msg).ToLocalChecked(); + + env->isolate()->ThrowException(exception); +} + } // namespace class NodeInspectorClient : public V8InspectorClient { @@ -728,6 +738,11 @@ bool Agent::StartIoThread() { if (io_ != nullptr) return true; + if (!parent_env_->should_create_inspector() && !client_) { + ThrowUninitializedInspectorError(parent_env_); + return false; + } + CHECK_NOT_NULL(client_); io_ = InspectorIo::Start(client_->getThreadHandle(), @@ -748,7 +763,13 @@ void Agent::Stop() { std::unique_ptr Agent::Connect( std::unique_ptr delegate, bool prevent_shutdown) { + if (!parent_env_->should_create_inspector() && !client_) { + ThrowUninitializedInspectorError(parent_env_); + return std::unique_ptr{}; + } + CHECK_NOT_NULL(client_); + int session_id = client_->connectFrontend(std::move(delegate), prevent_shutdown); return std::unique_ptr( @@ -758,6 +779,11 @@ std::unique_ptr Agent::Connect( std::unique_ptr Agent::ConnectToMainThread( std::unique_ptr delegate, bool prevent_shutdown) { + if (!parent_env_->should_create_inspector() && !client_) { + ThrowUninitializedInspectorError(parent_env_); + return std::unique_ptr{}; + } + CHECK_NOT_NULL(parent_handle_); CHECK_NOT_NULL(client_); auto thread_safe_delegate = @@ -767,6 +793,11 @@ std::unique_ptr Agent::ConnectToMainThread( } void Agent::WaitForDisconnect() { + if (!parent_env_->should_create_inspector() && !client_) { + ThrowUninitializedInspectorError(parent_env_); + return; + } + CHECK_NOT_NULL(client_); bool is_worker = parent_handle_ != nullptr; parent_handle_.reset(); @@ -912,6 +943,12 @@ void Agent::SetParentHandle( std::unique_ptr Agent::GetParentHandle( uint64_t thread_id, const std::string& url) { + if (!parent_env_->should_create_inspector() && !client_) { + ThrowUninitializedInspectorError(parent_env_); + return std::unique_ptr{}; + } + + CHECK_NOT_NULL(client_); if (!parent_handle_) { return client_->getWorkerManager()->NewParentHandle(thread_id, url); } else { @@ -920,11 +957,21 @@ std::unique_ptr Agent::GetParentHandle( } void Agent::WaitForConnect() { + if (!parent_env_->should_create_inspector() && !client_) { + ThrowUninitializedInspectorError(parent_env_); + return; + } + CHECK_NOT_NULL(client_); client_->waitForFrontend(); } std::shared_ptr Agent::GetWorkerManager() { + if (!parent_env_->should_create_inspector() && !client_) { + ThrowUninitializedInspectorError(parent_env_); + return std::unique_ptr{}; + } + CHECK_NOT_NULL(client_); return client_->getWorkerManager(); } diff --git a/src/node.h b/src/node.h index 5b1404ff8e290a..966edcd041be1b 100644 --- a/src/node.h +++ b/src/node.h @@ -438,7 +438,12 @@ enum Flags : uint64_t { // $HOME/.node_modules and $NODE_PATH. This is used by standalone apps that // do not expect to have their behaviors changed because of globally // installed modules. - kNoGlobalSearchPaths = 1 << 7 + kNoGlobalSearchPaths = 1 << 7, + // Controls whether or not the Environment should call V8Inspector::create(). + // This control is needed by embedders who may not want to initialize the V8 + // inspector in situations where one has already been created, + // e.g. Blink's in Chromium. + kNoCreateInspector = 1 << 9 }; } // namespace EnvironmentFlags