fix: raw_ptr destruction order in NodeBindings (#39762)

shell/common/node_bindings.cc

@@ -394,21 +394,11 @@ base::FilePath GetResourcesPath() {
   return exec_path.DirName().Append(FILE_PATH_LITERAL("resources"));
 #endif
 }
-
 }  // namespace
 
 NodeBindings::NodeBindings(BrowserEnvironment browser_env)
-    : browser_env_(browser_env) {
-  if (browser_env == BrowserEnvironment::kWorker) {
-    uv_loop_init(&worker_loop_);
-    uv_loop_ = &worker_loop_;
-  } else {
-    uv_loop_ = uv_default_loop();
-  }
-
-  // Interrupt embed polling when a handle is started.
-  uv_loop_configure(uv_loop_, UV_LOOP_INTERRUPT_ON_IO_CHANGE);
-}
+    : browser_env_{browser_env},
+      uv_loop_{InitEventLoop(browser_env, &worker_loop_)} {}
 
 NodeBindings::~NodeBindings() {
   // Quit the embed thread.
@@ -429,6 +419,24 @@ NodeBindings::~NodeBindings() {
     stop_and_close_uv_loop(uv_loop_);
 }
 
+// static
+uv_loop_t* NodeBindings::InitEventLoop(BrowserEnvironment browser_env,
+                                       uv_loop_t* worker_loop) {
+  uv_loop_t* event_loop = nullptr;
+
+  if (browser_env == BrowserEnvironment::kWorker) {
+    uv_loop_init(worker_loop);
+    event_loop = worker_loop;
+  } else {
+    event_loop = uv_default_loop();
+  }
+
+  // Interrupt embed polling when a handle is started.
+  uv_loop_configure(event_loop, UV_LOOP_INTERRUPT_ON_IO_CHANGE);
+
+  return event_loop;
+}
+
 void NodeBindings::RegisterBuiltinBindings() {
 #define V(modname) _register_##modname();
   if (IsBrowserProcess()) {

shell/common/node_bindings.h

@@ -132,9 +132,7 @@ class NodeBindings {
   void set_uv_env(node::Environment* env) { uv_env_ = env; }
   node::Environment* uv_env() const { return uv_env_; }
 
-  uv_loop_t* uv_loop() const { return uv_loop_; }
-
-  bool in_worker_loop() const { return uv_loop_ == &worker_loop_; }
+  [[nodiscard]] constexpr uv_loop_t* uv_loop() { return uv_loop_; }
 
   // disable copy
   NodeBindings(const NodeBindings&) = delete;
@@ -150,25 +148,36 @@ class NodeBindings {
   // Called to poll events in new thread.
   virtual void PollEvents() = 0;
 
-  // Run the libuv loop for once.
-  void UvRunOnce();
-
   // Make the main thread run libuv loop.
   void WakeupMainThread();
 
   // Interrupt the PollEvents.
   void WakeupEmbedThread();
 
+ private:
+  static uv_loop_t* InitEventLoop(BrowserEnvironment browser_env,
+                                  uv_loop_t* worker_loop);
+
+  // Run the libuv loop for once.
+  void UvRunOnce();
+
+  [[nodiscard]] constexpr bool in_worker_loop() const {
+    return browser_env_ == BrowserEnvironment::kWorker;
+  }
+
   // Which environment we are running.
   const BrowserEnvironment browser_env_;
 
-  // Current thread's MessageLoop.
-  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+  // Loop used when constructed in WORKER mode
+  uv_loop_t worker_loop_;
 
   // Current thread's libuv loop.
-  raw_ptr<uv_loop_t> uv_loop_;
+  // depends-on: worker_loop_
+  const raw_ptr<uv_loop_t> uv_loop_;
+
+  // Current thread's MessageLoop.
+  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
 
- private:
   // Choose a reasonable unique index that's higher than any Blink uses
   // and thus unlikely to collide with an existing index.
   static constexpr int kElectronContextEmbedderDataIndex =
@@ -192,9 +201,6 @@ class NodeBindings {
   // Whether the libuv loop has ended.
   bool embed_closed_ = false;
 
-  // Loop used when constructed in WORKER mode
-  uv_loop_t worker_loop_;
-
   // Dummy handle to make uv's loop not quit.
   UvHandle<uv_async_t> dummy_uv_handle_;
 

shell/common/node_bindings_linux.cc

@@ -10,7 +10,9 @@ namespace electron {
 
 NodeBindingsLinux::NodeBindingsLinux(BrowserEnvironment browser_env)
     : NodeBindings(browser_env), epoll_(epoll_create(1)) {
-  int backend_fd = uv_backend_fd(uv_loop_);
+  auto* const event_loop = uv_loop();
+
+  int backend_fd = uv_backend_fd(event_loop);
   struct epoll_event ev = {0};
   ev.events = EPOLLIN;
   ev.data.fd = backend_fd;
@@ -18,7 +20,9 @@ NodeBindingsLinux::NodeBindingsLinux(BrowserEnvironment browser_env)
 }
 
 void NodeBindingsLinux::PollEvents() {
-  int timeout = uv_backend_timeout(uv_loop_);
+  auto* const event_loop = uv_loop();
+
+  int timeout = uv_backend_timeout(event_loop);
 
   // Wait for new libuv events.
   int r;

shell/common/node_bindings_mac.cc

@@ -18,15 +18,17 @@ NodeBindingsMac::NodeBindingsMac(BrowserEnvironment browser_env)
     : NodeBindings(browser_env) {}
 
 void NodeBindingsMac::PollEvents() {
+  auto* const event_loop = uv_loop();
+
   struct timeval tv;
-  int timeout = uv_backend_timeout(uv_loop_);
+  int timeout = uv_backend_timeout(event_loop);
   if (timeout != -1) {
     tv.tv_sec = timeout / 1000;
     tv.tv_usec = (timeout % 1000) * 1000;
   }
 
   fd_set readset;
-  int fd = uv_backend_fd(uv_loop_);
+  int fd = uv_backend_fd(event_loop);
   FD_ZERO(&readset);
   FD_SET(fd, &readset);
 

shell/common/node_bindings_win.cc

@@ -13,34 +13,39 @@ namespace electron {
 
 NodeBindingsWin::NodeBindingsWin(BrowserEnvironment browser_env)
     : NodeBindings(browser_env) {
+  auto* const event_loop = uv_loop();
+
   // on single-core the io comp port NumberOfConcurrentThreads needs to be 2
   // to avoid cpu pegging likely caused by a busy loop in PollEvents
   if (base::SysInfo::NumberOfProcessors() == 1) {
-    // the expectation is the uv_loop_ has just been initialized
+    // the expectation is the event_loop has just been initialized
     // which makes iocp replacement safe
-    CHECK_EQ(0u, uv_loop_->active_handles);
-    CHECK_EQ(0u, uv_loop_->active_reqs.count);
+    CHECK_EQ(0u, event_loop->active_handles);
+    CHECK_EQ(0u, event_loop->active_reqs.count);
 
-    if (uv_loop_->iocp && uv_loop_->iocp != INVALID_HANDLE_VALUE)
-      CloseHandle(uv_loop_->iocp);
-    uv_loop_->iocp = CreateIoCompletionPort(INVALID_HANDLE_VALUE, NULL, 0, 2);
+    if (event_loop->iocp && event_loop->iocp != INVALID_HANDLE_VALUE)
+      CloseHandle(event_loop->iocp);
+    event_loop->iocp = CreateIoCompletionPort(INVALID_HANDLE_VALUE, NULL, 0, 2);
   }
 }
 
 void NodeBindingsWin::PollEvents() {
+  auto* const event_loop = uv_loop();
+
   // If there are other kinds of events pending, uv_backend_timeout will
   // instruct us not to wait.
   DWORD bytes, timeout;
   ULONG_PTR key;
   OVERLAPPED* overlapped;
 
-  timeout = uv_backend_timeout(uv_loop_);
+  timeout = uv_backend_timeout(event_loop);
 
-  GetQueuedCompletionStatus(uv_loop_->iocp, &bytes, &key, &overlapped, timeout);
+  GetQueuedCompletionStatus(event_loop->iocp, &bytes, &key, &overlapped,
+                            timeout);
 
   // Give the event back so libuv can deal with it.
   if (overlapped != NULL)
-    PostQueuedCompletionStatus(uv_loop_->iocp, bytes, key, overlapped);
+    PostQueuedCompletionStatus(event_loop->iocp, bytes, key, overlapped);
 }
 
 // static

shell/renderer/electron_renderer_client.cc

@@ -27,10 +27,10 @@
 namespace electron {
 
 ElectronRendererClient::ElectronRendererClient()
-    : node_bindings_(
-          NodeBindings::Create(NodeBindings::BrowserEnvironment::kRenderer)),
-      electron_bindings_(
-          std::make_unique<ElectronBindings>(node_bindings_->uv_loop())) {}
+    : node_bindings_{NodeBindings::Create(
+          NodeBindings::BrowserEnvironment::kRenderer)},
+      electron_bindings_{
+          std::make_unique<ElectronBindings>(node_bindings_->uv_loop())} {}
 
 ElectronRendererClient::~ElectronRendererClient() = default;
 

shell/renderer/electron_renderer_client.h

@@ -51,8 +51,8 @@ class ElectronRendererClient : public RendererClientBase {
   // Whether the node integration has been initialized.
   bool node_integration_initialized_ = false;
 
-  std::unique_ptr<NodeBindings> node_bindings_;
-  std::unique_ptr<ElectronBindings> electron_bindings_;
+  const std::unique_ptr<NodeBindings> node_bindings_;
+  const std::unique_ptr<ElectronBindings> electron_bindings_;
 
   // The node::Environment::GetCurrent API does not return nullptr when it
   // is called for a context without node::Environment, so we have to keep