Skip to content

Commit fdb8864

Browse files
nbronsonmahmood-openai
authored andcommitted
fix deadlock in RCU retire when rcu_reader active
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
1 parent 31f16ed commit fdb8864

File tree

3 files changed

+30
-2
lines changed

3 files changed

+30
-2
lines changed

CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,6 +1038,7 @@ if (BUILD_TESTS OR BUILD_BENCHMARKS)
10381038
SOURCES LifoSemTests.cpp
10391039
TEST synchronization_relaxed_atomic_test WINDOWS_DISABLED
10401040
SOURCES RelaxedAtomicTest.cpp
1041+
TEST synchronization_rcu_test SOURCES RcuTest.cpp
10411042
TEST synchronization_rw_spin_lock_test SOURCES RWSpinLockTest.cpp
10421043
TEST synchronization_semaphore_test WINDOWS_DISABLED
10431044
SOURCES SemaphoreTest.cpp

folly/synchronization/Rcu.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -384,8 +384,12 @@ class rcu_domain {
384384
syncTime, time, std::memory_order_relaxed)) {
385385
list_head finished;
386386
{
387-
std::lock_guard<std::mutex> g(syncMutex_);
388-
half_sync(false, finished);
387+
// synchronize() blocks while holding syncMutex_,
388+
// so we must not wait for syncMutex_
389+
std::unique_lock<std::mutex> g(syncMutex_, std::try_to_lock);
390+
if (g) {
391+
half_sync(false, finished);
392+
}
389393
}
390394
// callbacks are called outside of syncMutex_
391395
finished.forEach([&](list_node* item) {

folly/synchronization/test/RcuTest.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,3 +330,26 @@ TEST(RcuTest, DeeplyNestedReaders) {
330330
rcu_synchronize();
331331
delete int_ptr.exchange(nullptr, std::memory_order_acq_rel);
332332
}
333+
334+
TEST(RcuTest, RetireUnderLock) {
335+
using namespace std::chrono;
336+
337+
// syncTimePeriod is 3.2s
338+
auto deadline = steady_clock::now() + seconds(4);
339+
340+
std::thread t1([&] {
341+
while (steady_clock::now() < deadline) {
342+
rcu_synchronize();
343+
}
344+
});
345+
346+
std::thread t2([&] {
347+
while (steady_clock::now() < deadline) {
348+
std::scoped_lock<rcu_domain> g(rcu_default_domain());
349+
rcu_default_domain().call([] {});
350+
}
351+
});
352+
353+
t1.join();
354+
t2.join();
355+
}

0 commit comments

Comments
 (0)