Facebook/react-native
1
0
Fork 0
mirror of https://github.com/facebook/react-native.git synced 2025-11-01 09:14:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix TaskDispatchThread (#53961)

Browse Source 
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
This commit is contained in:
Nurtau Toganbay
2025-09-26 08:54:24 -07:00
committed by Facebook GitHub Bot
parent 2d2011c7ae
commit ad5949ffd6
2 changed files with 13 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/react-native/ReactCxxPlatform/react/threading/TaskDispatchThread.cpp
+1
View File
@@ -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
packages/react-native/ReactCxxPlatform/react/threading/tests/TaskDispatchThreadTests.cpp
+12
View File
@@ -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<int> counter = std::make_unique<int>(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<int> 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