src: allow preventing InitializeInspector in env

PR-URL: #35025 Reviewed-By: David Carlier <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Apr 24, 2022 · a867867 · a867867
1 parent 101f915
commit a867867
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 7 deletions.
diff --git 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<InspectorParentHandleImpl*>(
-            inspector_parent_handle.get())->impl));
-  } else {
-    env->InitializeInspector({});
+  if (env->should_create_inspector()) {
+    if (inspector_parent_handle) {
+      env->InitializeInspector(
+          std::move(static_cast<InspectorParentHandleImpl*>(
+              inspector_parent_handle.get())->impl));
+    } else {
+      env->InitializeInspector({});
+    }
   }
 #endif
 

diff --git 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
@@ -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
@@ -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<Value> 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<InspectorSession> Agent::Connect(
     std::unique_ptr<InspectorSessionDelegate> delegate,
     bool prevent_shutdown) {
+  if (!parent_env_->should_create_inspector() && !client_) {
+    ThrowUninitializedInspectorError(parent_env_);
+    return std::unique_ptr<InspectorSession>{};
+  }
+
   CHECK_NOT_NULL(client_);
+
   int session_id = client_->connectFrontend(std::move(delegate),
                                             prevent_shutdown);
   return std::unique_ptr<InspectorSession>(
@@ -758,6 +779,11 @@ std::unique_ptr<InspectorSession> Agent::Connect(
 std::unique_ptr<InspectorSession> Agent::ConnectToMainThread(
     std::unique_ptr<InspectorSessionDelegate> delegate,
     bool prevent_shutdown) {
+  if (!parent_env_->should_create_inspector() && !client_) {
+    ThrowUninitializedInspectorError(parent_env_);
+    return std::unique_ptr<InspectorSession>{};
+  }
+
   CHECK_NOT_NULL(parent_handle_);
   CHECK_NOT_NULL(client_);
   auto thread_safe_delegate =
@@ -767,6 +793,11 @@ std::unique_ptr<InspectorSession> 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<ParentInspectorHandle> Agent::GetParentHandle(
     uint64_t thread_id, const std::string& url) {
+  if (!parent_env_->should_create_inspector() && !client_) {
+    ThrowUninitializedInspectorError(parent_env_);
+    return std::unique_ptr<ParentInspectorHandle>{};
+  }
+
+  CHECK_NOT_NULL(client_);
   if (!parent_handle_) {
     return client_->getWorkerManager()->NewParentHandle(thread_id, url);
   } else {
@@ -920,11 +957,21 @@ std::unique_ptr<ParentInspectorHandle> 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<WorkerManager> Agent::GetWorkerManager() {
+  if (!parent_env_->should_create_inspector() && !client_) {
+    ThrowUninitializedInspectorError(parent_env_);
+    return std::unique_ptr<WorkerManager>{};
+  }
+
   CHECK_NOT_NULL(client_);
   return client_->getWorkerManager();
 }

diff --git 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