Skip to content

Commit

Permalink
fix deadlock in RCU retire when rcu_reader active
Browse files Browse the repository at this point in the history
Summary:
Every 3.2 seconds RCU retire tries to opportunistically
drain the queue of pending work.  If this happens while
the current thread is holding an rcu_reader and another
thread is currently running synchronize(), a deadlock will
occur.  This diff adds a unit test for the behavior and
fixes the problem.

Test Plan:
* unit test that reproduces the problem
* unit test passes with the fix
  • Loading branch information
nbronson committed Apr 19, 2023
1 parent 535a8ed commit 035ad2a
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 2 deletions.
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,7 @@ if (BUILD_TESTS)
TEST baton_test SOURCES BatonTest.cpp
TEST call_once_test SOURCES CallOnceTest.cpp
TEST lifo_sem_test SOURCES LifoSemTests.cpp
TEST rcu_test SOURCES RcuTest.cpp
TEST rw_spin_lock_test SOURCES RWSpinLockTest.cpp
TEST semaphore_test SOURCES SemaphoreTest.cpp

Expand Down
8 changes: 6 additions & 2 deletions folly/synchronization/Rcu-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,12 @@ void rcu_domain<Tag>::retire(list_node* node) noexcept {
syncTime, time, std::memory_order_relaxed)) {
list_head finished;
{
std::lock_guard<std::mutex> g(syncMutex_);
half_sync(false, finished);
// synchronize() blocks while holding syncMutex_,
// so we must not wait for syncMutex_
std::unique_lock<std::mutex> g(syncMutex_, std::try_to_lock);
if (g) {
half_sync(false, finished);
}
}
// callbacks are called outside of syncMutex_
finished.forEach(
Expand Down
23 changes: 23 additions & 0 deletions folly/synchronization/test/RcuTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -308,3 +308,26 @@ TEST(RcuTest, Tsan) {
t2.join();
EXPECT_EQ(data, 2);
}

TEST(RcuTest, RetireUnderLock) {
using namespace std::chrono;

// syncTimePeriod is 3.2s
auto deadline = steady_clock::now() + seconds(4);

std::thread t1([&] {
while (steady_clock::now() < deadline) {
synchronize_rcu();
}
});

std::thread t2([&] {
while (steady_clock::now() < deadline) {
rcu_reader g;
rcu_default_domain()->call([] {});
}
});

t1.join();
t2.join();
}

0 comments on commit 035ad2a

Please sign in to comment.