fix: close the utilityProcess port before its descriptor is destroyed (#55233)

* fix: close the utilityProcess port before its descriptor is destroyed

A UtilityProcessWrapper that is still alive when the heap is torn down at
exit, for example one whose child had not finished launching when
ElectronBrowserMainParts shut the others down, was destroyed with its
postMessage port still lent to the mojo::Connector. Debug and sanitizer
builds then hit DCHECK(handle_.is_valid()) in MessagePortDescriptor::Reset().

Move the connector and port descriptor onto a NativePeer. Its
TearDownNative() takes the handle back and resets the descriptor, so the
port is closed at MicrotasksRunner shutdown or after collection, never
during cppgc sweeping. The exit-time order (dispatch pending messages,
close the port, emit 'exit') is unchanged.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LPMvJBQiyP6b7QjatACmnS

* fix: close the utilityProcess port in the destructor and shut down launching wrappers

Per review, replace the PortLifecycle native peer with a smaller fix:

- ~UtilityProcessWrapper closes the connector port and any stdio read fds
  not yet handed to JS. Both are non-GC members, so this is allowed during
  cppgc sweeping, and it fixes the DCHECK in MessagePortDescriptor::Reset()
  for a wrapper still alive at heap teardown.
- Bind the connector error handler with base::Unretained: the connector is
  owned by the wrapper and destroying it cancels the handler.
- Drop the connector_->is_valid() check in CloseConnectorPort();
  GiveDisentangledHandle() replaces an invalid handle with a dangling one, so
  the port is always reset.
- Track wrappers from construction instead of from launch, and add
  UtilityProcessWrapper::ShutdownAll(), so PostMainMessageLoopRun() also
  shuts down wrappers whose child is still launching before Node.js stops.
  FromProcessId() ignores a null pid.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LPMvJBQiyP6b7QjatACmnS

---------

Co-authored-by: Claude <noreply@anthropic.com>

shell/browser/api/electron_api_utility_process.cc

@@ -6,8 +6,8 @@
 
 #include <array>
 #include <map>
-#include <unordered_map>
 #include <utility>
+#include <vector>
 
 #include "base/functional/bind.h"
 #include "base/location.h"
@@ -71,21 +71,24 @@ namespace electron {
 
 namespace {
 
-// Maps process IDs to their UtilityProcessWrapper instances.
+// Tracks every UtilityProcessWrapper from construction until termination,
+// including ones whose child has not launched yet and so has no pid.
 struct UtilityProcessRegistry {
-  void Add(base::ProcessId pid, api::UtilityProcessWrapper* wrapper) {
-    map_.emplace(pid, wrapper);
+  using Entries =
+      std::vector<cppgc::WeakPersistent<api::UtilityProcessWrapper>>;
+
+  void Add(api::UtilityProcessWrapper* wrapper) {
+    wrappers_.emplace_back(wrapper);
   }
-  void Remove(base::ProcessId pid) { map_.erase(pid); }
-  api::UtilityProcessWrapper* Lookup(base::ProcessId pid) {
-    auto it = map_.find(pid);
-    return it != map_.end() ? it->second.Get() : nullptr;
+  void Remove(api::UtilityProcessWrapper* wrapper) {
+    std::erase_if(wrappers_, [wrapper](const auto& entry) {
+      return !entry || entry.Get() == wrapper;
+    });
   }
+  const Entries& wrappers() const { return wrappers_; }
 
  private:
-  std::unordered_map<base::ProcessId,
-                     cppgc::WeakPersistent<api::UtilityProcessWrapper>>
-      map_;
+  Entries wrappers_;
 };
 
 UtilityProcessRegistry& GetAllUtilityProcessWrappers() {
@@ -113,6 +116,9 @@ UtilityProcessWrapper::UtilityProcessWrapper(
     bool disclaim_responsibility,
     Session* session)
     : create_network_observer_(create_network_observer), session_(session) {
+  // Registered before anything can fail, so that browser shutdown also reaches
+  // a wrapper whose child is still launching. HandleTermination() removes it.
+  GetAllUtilityProcessWrappers().Add(this);
   auto& allocation_handle =
       JavascriptEnvironment::GetIsolate()->GetCppHeap()->GetAllocationHandle();
 #if BUILDFLAG(IS_WIN)
@@ -272,9 +278,10 @@ UtilityProcessWrapper::UtilityProcessWrapper(
       mojo::Connector::SINGLE_THREADED_SEND,
       base::SingleThreadTaskRunner::GetCurrentDefault());
   connector_->set_incoming_receiver(this);
+  // |connector_| is owned by this wrapper and only ever reset, never moved, so
+  // destroying it cancels the handler.
   connector_->set_connection_error_handler(base::BindOnce(
-      &UtilityProcessWrapper::CloseConnectorPort,
-      gin::WrapPersistent(weak_factory_.GetWeakCell(allocation_handle))));
+      &UtilityProcessWrapper::CloseConnectorPort, base::Unretained(this)));
 
   params->url_loader_factory_params = CreateURLLoaderFactoryParams();
   node_service_remote_->Initialize(std::move(params),
@@ -290,6 +297,12 @@ UtilityProcessWrapper::UtilityProcessWrapper(
 UtilityProcessWrapper::~UtilityProcessWrapper() {
   content::ServiceProcessHost::RemoveObserver(this);
   content::BrowserChildProcessObserver::Remove(this);
+  // A wrapper that never reached HandleTermination(), e.g. one still alive at
+  // heap teardown, still owns its port and stdio read ends. Both are native,
+  // non-GC members. A port destroyed while still lent to |connector_| fails a
+  // DCHECK in MessagePortDescriptor::Reset().
+  CloseConnectorPort();
+  CloseStdioReadFds();
 }
 
 void UtilityProcessWrapper::OnServiceProcessLaunch(
@@ -298,7 +311,6 @@ void UtilityProcessWrapper::OnServiceProcessLaunch(
     return;
   DCHECK(node_service_remote_.is_connected());
   pid_ = process.Pid();
-  GetAllUtilityProcessWrappers().Add(pid_, this);
   // Remember the child process id: the process has already gone from the
   // ChildProcessData that BrowserChildProcessCrashed/Killed receive.
   for (content::BrowserChildProcessHostIterator it(
@@ -328,8 +340,7 @@ void UtilityProcessWrapper::HandleTermination(uint32_t exit_code) {
     return;
   terminated_ = true;
 
-  if (pid_ != base::kNullProcessId)
-    GetAllUtilityProcessWrappers().Remove(pid_);
+  GetAllUtilityProcessWrappers().Remove(this);
 
   pid_ = base::kNullProcessId;
   child_process_id_ = content::ChildProcessId();
@@ -418,7 +429,9 @@ void UtilityProcessWrapper::CloseStdioReadFds() {
 }
 
 void UtilityProcessWrapper::CloseConnectorPort() {
-  if (!connector_closed_ && connector_ && connector_->is_valid()) {
+  // GiveDisentangledHandle() replaces an invalid handle with a dangling one, so
+  // the port is always reset, even if the connector's pipe is gone.
+  if (!connector_closed_ && connector_) {
     host_port_.GiveDisentangledHandle(connector_->PassMessagePipe());
     connector_ = nullptr;
     host_port_.Reset();
@@ -604,8 +617,27 @@ void UtilityProcessWrapper::BindAIManager(
 // static
 UtilityProcessWrapper* UtilityProcessWrapper::FromProcessId(
     base::ProcessId pid) {
-  auto* utility_process_wrapper = GetAllUtilityProcessWrappers().Lookup(pid);
-  return utility_process_wrapper ? utility_process_wrapper : nullptr;
+  // Wrappers whose child has not launched yet have no pid.
+  if (pid == base::kNullProcessId)
+    return nullptr;
+  for (const auto& wrapper : GetAllUtilityProcessWrappers().wrappers()) {
+    if (wrapper && wrapper->pid_ == pid)
+      return wrapper.Get();
+  }
+  return nullptr;
+}
+
+// static
+void UtilityProcessWrapper::ShutdownAll() {
+  // Shutdown() removes each wrapper from the registry and runs 'exit'
+  // listeners, so iterate over a copy that keeps the wrappers alive.
+  std::vector<cppgc::Persistent<UtilityProcessWrapper>> wrappers;
+  for (const auto& wrapper : GetAllUtilityProcessWrappers().wrappers()) {
+    if (wrapper)
+      wrappers.emplace_back(wrapper.Get());
+  }
+  for (const auto& wrapper : wrappers)
+    wrapper->Shutdown(0 /* exit_code */);
 }
 
 // static

shell/browser/api/electron_api_utility_process.h

@@ -72,6 +72,9 @@ class UtilityProcessWrapper final
 
   static UtilityProcessWrapper* Create(gin::Arguments* args);
   static UtilityProcessWrapper* FromProcessId(base::ProcessId pid);
+  // Shuts down every wrapper that has not terminated, including ones whose
+  // child is still launching. Called before Node.js stops at browser shutdown.
+  static void ShutdownAll();
 
 #if BUILDFLAG(ENABLE_PROMPT_API)
   void BindAIManager(std::optional<int32_t> web_contents_id,

shell/browser/electron_browser_main_parts.cc

@@ -24,16 +24,13 @@
 #include "chrome/browser/ui/color/chrome_color_mixers.h"
 #include "chrome/common/chrome_switches.h"
 #include "content/public/browser/browser_child_process_host_delegate.h"
-#include "content/public/browser/browser_child_process_host_iterator.h"
 #include "content/public/browser/browser_main_runner.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/browser/child_process_data.h"
 #include "content/public/browser/device_service.h"
 #include "content/public/browser/download_manager.h"
 #include "content/public/browser/web_ui_controller_factory.h"
 #include "content/public/common/content_switches.h"
-#include "content/public/common/process_type.h"
 #include "content/public/common/result_codes.h"
 #include "electron/buildflags/buildflags.h"
 #include "electron/fuses.h"
@@ -704,26 +701,9 @@ void ElectronBrowserMainParts::PostMainMessageLoopRun() {
   // use case.
   // https://source.chromium.org/chromium/chromium/src/+/main:content/browser/browser_main_loop.cc;l=1086-1108
   //
-  // The following logic is based on
-  // https://source.chromium.org/chromium/chromium/src/+/main:content/browser/browser_process_io_thread.cc;l=127-159
-  //
-  // Although content::BrowserChildProcessHostIterator is only to be called from
-  // IO thread, it is safe to call from PostMainMessageLoopRun because thread
-  // restrictions have been lifted.
-  // https://source.chromium.org/chromium/chromium/src/+/main:content/browser/browser_main_loop.cc;l=1062-1078
-  for (content::BrowserChildProcessHostIterator it(
-           content::PROCESS_TYPE_UTILITY);
-       !it.Done(); ++it) {
-    if (it.GetDelegate()->GetServiceName() == node::mojom::NodeService::Name_) {
-      const base::Process& process = it.GetProcess();
-      if (!process.IsValid())
-        continue;
-      auto* utility_process_wrapper =
-          api::UtilityProcessWrapper::FromProcessId(process.Pid());
-      if (utility_process_wrapper)
-        utility_process_wrapper->Shutdown(0 /* exit_code */);
-    }
-  }
+  // This includes wrappers whose child is still launching; otherwise they
+  // would only be torn down with the heap, after Node.js has stopped.
+  api::UtilityProcessWrapper::ShutdownAll();
 
   // Destroy node platform after all destructors_ are executed, as they may
   // invoke Node/V8 APIs inside them.