Make runSync more robust (#55348)

Summary:
Pull Request resolved: #55348

Changelog: [Internal]

Reviewed By: Nurtau

Differential Revision: D91779318

fbshipit-source-id: d196db8293889ecef9fbf48af6150da04f809325

Author: javache

packages/react-native/ReactCxxPlatform/react/threading/MessageQueueThreadImpl.cpp

@@ -15,8 +15,7 @@ void MessageQueueThreadImpl::runOnQueue(std::function<void()>&& runnable) {
   if (!taskDispatchThread_.isRunning()) {
     return;
   }
-  taskDispatchThread_.runAsync(
-      [runnable = std::move(runnable)]() noexcept { runnable(); });
+  taskDispatchThread_.runAsync(std::move(runnable));
 }
 
 void MessageQueueThreadImpl::runOnQueueSync(std::function<void()>&& runnable) {
@@ -26,8 +25,7 @@ void MessageQueueThreadImpl::runOnQueueSync(std::function<void()>&& runnable) {
   if (taskDispatchThread_.isOnThread()) {
     runnable();
   } else {
-    taskDispatchThread_.runSync(
-        [runnable = std::move(runnable)]() noexcept { runnable(); });
+    taskDispatchThread_.runSync(std::move(runnable));
   }
 }
 

packages/react-native/ReactCxxPlatform/react/threading/TaskDispatchThread.cpp

@@ -9,6 +9,7 @@
 
 #include <folly/portability/SysResource.h>
 #include <folly/system/ThreadName.h>
+#include <cassert>
 #include <chrono>
 #include <future>
 #include <utility>
@@ -67,27 +68,30 @@ bool TaskDispatchThread::isRunning() noexcept {
 void TaskDispatchThread::runAsync(
     TaskFn&& task,
     std::chrono::milliseconds delayMs) noexcept {
+  std::lock_guard<std::mutex> guard(mutex_);
   if (!running_) {
     return;
   }
-  std::lock_guard<std::mutex> guard(mutex_);
   auto dispatchTime = std::chrono::system_clock::now() + delayMs;
   queue_.emplace(dispatchTime, std::move(task));
   loopCv_.notify_one();
 }
 
 void TaskDispatchThread::runSync(TaskFn&& task) noexcept {
-  if (!running_) {
-    return;
-  }
-  std::promise<void> promise;
-  runAsync([&]() {
-    if (running_) {
-      task();
-    }
-    promise.set_value();
-  });
-  promise.get_future().wait();
+  // Calling runSync from the dispatch thread would deadlock, as the thread
+  // would wait for itself to execute the task it just queued.
+  assert(!isOnThread());
+
+  // Use shared_ptr to ensure the promise outlives both the caller and the
+  // queued task. The destructor guard ensures the promise is fulfilled even
+  // if the task is destroyed without executing (e.g., when quit() clears the
+  // queue during shutdown).
+  auto promise = std::make_shared<std::promise<void>>();
+  auto guard = std::shared_ptr<void>(
+      nullptr, [promise](void*) { promise->set_value(); });
+
+  runAsync([guard = std::move(guard), task = std::move(task)]() { task(); });
+  promise->get_future().wait();
 }
 
 void TaskDispatchThread::quit() noexcept {

packages/react-native/ReactCxxPlatform/react/threading/tests/TaskDispatchThreadTests.cpp

@@ -159,4 +159,40 @@ TEST_F(TaskDispatchThreadTest, SyncTaskShouldntBeBlockedDueToDelayedTask) {
   dispatcher->runSync([&] { counter++; });
   EXPECT_EQ(counter.load(), 1);
 }
+
+// Test: runSync should not block forever when quit() is called concurrently
+// This tests the fix where the RAII guard ensures the promise is fulfilled
+// even if the task is destroyed without executing during shutdown.
+TEST_F(TaskDispatchThreadTest, RunSyncDoesNotBlockWhenQuitCalledConcurrently) {
+  for (int i = 0; i < 100; ++i) {
+    auto localDispatcher = std::make_unique<TaskDispatchThread>();
+
+    std::atomic<bool> runSyncCompleted{false};
+
+    // Block the dispatcher thread with a slow task
+    localDispatcher->runAsync(
+        [&] { std::this_thread::sleep_for(std::chrono::milliseconds(10)); });
+
+    // Start runSync from another thread - it will queue a task and wait
+    std::thread syncThread([&] {
+      localDispatcher->runSync([&] {});
+      runSyncCompleted = true;
+    });
+
+    // Concurrently call quit() - this should cause the queued task to be
+    // destroyed without executing, but runSync should still return (not block
+    // forever) because the RAII guard fulfills the promise on destruction.
+    std::this_thread::sleep_for(std::chrono::milliseconds(5));
+    localDispatcher->quit();
+
+    // If the fix works, syncThread should complete. If not, this would hang.
+    syncThread.join();
+
+    // runSync should have completed (either task ran or was skipped due to
+    // shutdown)
+    EXPECT_TRUE(runSyncCompleted.load());
+
+    localDispatcher.reset();
+  }
+}
 } // namespace facebook::react