Browse Source

fix: uv_walk crash on web worker close (#24463)

* fix: uv_walk crash on web worker close

* Use DCHECK_EQ

Co-authored-by: Shelley Vohr <[email protected]>
trop[bot] 4 years ago
parent
commit
b762563257
2 changed files with 23 additions and 19 deletions
  1. 21 19
      shell/common/node_bindings.cc
  2. 2 0
      shell/common/node_bindings.h

+ 21 - 19
shell/common/node_bindings.cc

@@ -101,25 +101,25 @@ ELECTRON_DESKTOP_CAPTURER_MODULE(V)
 namespace {
 
 void stop_and_close_uv_loop(uv_loop_t* loop) {
-  // Close any active handles
   uv_stop(loop);
-  uv_walk(
-      loop,
-      [](uv_handle_t* handle, void*) {
-        if (!uv_is_closing(handle)) {
-          uv_close(handle, nullptr);
-        }
-      },
-      nullptr);
-
-  // Run the loop to let it finish all the closing handles
-  // NB: after uv_stop(), uv_run(UV_RUN_DEFAULT) returns 0 when that's done
-  for (;;)
-    if (!uv_run(loop, UV_RUN_DEFAULT))
-      break;
+  int error = uv_loop_close(loop);
+
+  while (error) {
+    uv_run(loop, UV_RUN_DEFAULT);
+    uv_stop(loop);
+    uv_walk(
+        loop,
+        [](uv_handle_t* handle, void*) {
+          if (!uv_is_closing(handle)) {
+            uv_close(handle, nullptr);
+          }
+        },
+        nullptr);
+    uv_run(loop, UV_RUN_DEFAULT);
+    error = uv_loop_close(loop);
+  }
 
-  DCHECK(!uv_loop_alive(loop));
-  uv_loop_close(loop);
+  DCHECK_EQ(error, 0);
 }
 
 bool g_is_initialized = false;
@@ -226,6 +226,7 @@ NodeBindings::~NodeBindings() {
   // Quit the embed thread.
   embed_closed_ = true;
   uv_sem_post(&embed_sem_);
+
   WakeupEmbedThread();
 
   // Wait for everything to be done.
@@ -236,7 +237,7 @@ NodeBindings::~NodeBindings() {
   uv_close(reinterpret_cast<uv_handle_t*>(&dummy_uv_handle_), nullptr);
 
   // Clean up worker loop
-  if (uv_loop_ == &worker_loop_)
+  if (in_worker_loop())
     stop_and_close_uv_loop(uv_loop_);
 }
 
@@ -436,7 +437,8 @@ void NodeBindings::WakeupMainThread() {
 }
 
 void NodeBindings::WakeupEmbedThread() {
-  uv_async_send(&dummy_uv_handle_);
+  if (!in_worker_loop())
+    uv_async_send(&dummy_uv_handle_);
 }
 
 // static

+ 2 - 0
shell/common/node_bindings.h

@@ -59,6 +59,8 @@ class NodeBindings {
 
   uv_loop_t* uv_loop() const { return uv_loop_; }
 
+  bool in_worker_loop() const { return uv_loop_ == &worker_loop_; }
+
  protected:
   explicit NodeBindings(BrowserEnvironment browser_env);