From ad5949ffd69743cfe0ffebf1f045518df7c7c38d Mon Sep 17 00:00:00 2001 From: Nurtau Toganbay Date: Fri, 26 Sep 2025 08:54:24 -0700 Subject: [PATCH] Fix TaskDispatchThread (#53961) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/53961 Changelog: [Internal] Fixed a bug in TaskDispatchThread. To understand bug, let me show an example. The task is scheduled to run after several seconds. It reaches loopCv_.wait_until() and waits there. There are 2 bad scenario: 1. wait_until spuriously wakes up and async task runs even before its scheduled time 2. new task is added with runSync(). New task gets added to the queue and loopCv_ is notified. Async task will run before its scheduled time and even worse queue_.pop() will remove sync task. Therefore runSync will be blocked for forever. To fix this bug, we need to add `continue` after wait_until(). Also I added new test to prevent this bug in future. Reviewed By: rshest Differential Revision: D83345047 fbshipit-source-id: 37962613a123a123c0e110426ae782effe5a81c1 --- .../react/threading/TaskDispatchThread.cpp | 1 + .../threading/tests/TaskDispatchThreadTests.cpp | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/packages/react-native/ReactCxxPlatform/react/threading/TaskDispatchThread.cpp b/packages/react-native/ReactCxxPlatform/react/threading/TaskDispatchThread.cpp index f57774e8890..466d95b07f7 100644 --- a/packages/react-native/ReactCxxPlatform/react/threading/TaskDispatchThread.cpp +++ b/packages/react-native/ReactCxxPlatform/react/threading/TaskDispatchThread.cpp @@ -118,6 +118,7 @@ void TaskDispatchThread::loop() noexcept { if (task.dispatchTime > now) { // Wait until the scheduled task time, if delayed loopCv_.wait_until(lock, task.dispatchTime); + continue; } } else { // Shutting down, skip all the remaining tasks diff --git a/packages/react-native/ReactCxxPlatform/react/threading/tests/TaskDispatchThreadTests.cpp b/packages/react-native/ReactCxxPlatform/react/threading/tests/TaskDispatchThreadTests.cpp index 8bd61a703d5..867f1815df2 100644 --- a/packages/react-native/ReactCxxPlatform/react/threading/tests/TaskDispatchThreadTests.cpp +++ b/packages/react-native/ReactCxxPlatform/react/threading/tests/TaskDispatchThreadTests.cpp @@ -128,10 +128,12 @@ TEST_F(TaskDispatchThreadTest, RunAsyncFromMultipleThreads) { EXPECT_EQ(counter.load(), 3); } +// Test: quit() shouldn't block if it is called inside loop thread TEST_F(TaskDispatchThreadTest, QuitInTaskShouldntBeBlockedForever) { dispatcher->runSync([&] { dispatcher->quit(); }); } +// Test: quit() should wait for already running task in the thread TEST_F(TaskDispatchThreadTest, QuitShouldWaitAlreadyRunningTask) { { std::unique_ptr counter = std::make_unique(0); @@ -147,4 +149,14 @@ TEST_F(TaskDispatchThreadTest, QuitShouldWaitAlreadyRunningTask) { // forcing dispatcher to join thread dispatcher.reset(); } + +// Test: sync tasks shouldn't be blocked for forever due to delayed task +TEST_F(TaskDispatchThreadTest, SyncTaskShouldntBeBlockedDueToDelayedTask) { + std::atomic counter = 0; + constexpr int kHugeDelay = 100000; + dispatcher->runAsync([&] {}, std::chrono::seconds(kHugeDelay)); + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + dispatcher->runSync([&] { counter++; }); + EXPECT_EQ(counter.load(), 1); +} } // namespace facebook::react