Commit eab7000d authored by Maged Michael's avatar Maged Michael Committed by Facebook Github Bot

Fix data race in Futex<EmulatedFutexAtomic>

Summary: Fixed a data race between an atomic store by the waker and a nonatomic memcpy by the waiter.

Reviewed By: nbronson

Differential Revision: D4572410

fbshipit-source-id: 3982ca433e0f628636916516e35aeb7738ae030f
parent f54a0bdb
...@@ -226,21 +226,25 @@ int emulatedFutexWake(void* addr, int count, uint32_t waitMask) { ...@@ -226,21 +226,25 @@ int emulatedFutexWake(void* addr, int count, uint32_t waitMask) {
return numAwoken; return numAwoken;
} }
template <typename F>
FutexResult emulatedFutexWaitImpl( FutexResult emulatedFutexWaitImpl(
void* addr, F* futex,
uint32_t expected, uint32_t expected,
time_point<system_clock>* absSystemTime, time_point<system_clock>* absSystemTime,
time_point<steady_clock>* absSteadyTime, time_point<steady_clock>* absSteadyTime,
uint32_t waitMask) { uint32_t waitMask) {
static_assert(
std::is_same<F, Futex<std::atomic>>::value ||
std::is_same<F, Futex<EmulatedFutexAtomic>>::value,
"Type F must be either Futex<std::atomic> or Futex<EmulatedFutexAtomic>");
void* addr = static_cast<void*>(futex);
auto& bucket = EmulatedFutexBucket::bucketFor(addr); auto& bucket = EmulatedFutexBucket::bucketFor(addr);
EmulatedFutexWaitNode node(addr, waitMask); EmulatedFutexWaitNode node(addr, waitMask);
{ {
std::unique_lock<std::mutex> bucketLock(bucket.mutex_); std::unique_lock<std::mutex> bucketLock(bucket.mutex_);
uint32_t actual; if (futex->load(std::memory_order_relaxed) != expected) {
memcpy(&actual, addr, sizeof(uint32_t));
if (actual != expected) {
return FutexResult::VALUE_CHANGED; return FutexResult::VALUE_CHANGED;
} }
......
...@@ -182,6 +182,21 @@ void run_steady_clock_test() { ...@@ -182,6 +182,21 @@ void run_steady_clock_test() {
EXPECT_TRUE(A <= B && B <= C); EXPECT_TRUE(A <= B && B <= C);
} }
template <template <typename> class Atom>
void run_wake_blocked_test() {
Futex<Atom> f(0);
auto thr = DSched::thread([&] { EXPECT_TRUE(f.futexWait(0)); });
// A sleep here guarantees to a large extent that 'thr' will execute
// futexWait before we wake it up, thus testing late futexWake.
std::this_thread::sleep_for(std::chrono::milliseconds(2));
f.store(1);
f.futexWake(1);
DSched::join(thr);
}
TEST(Futex, clock_source) { TEST(Futex, clock_source) {
run_system_clock_test(); run_system_clock_test();
...@@ -207,3 +222,11 @@ TEST(Futex, basic_deterministic) { ...@@ -207,3 +222,11 @@ TEST(Futex, basic_deterministic) {
run_basic_tests<DeterministicAtomic>(); run_basic_tests<DeterministicAtomic>();
run_wait_until_tests<DeterministicAtomic>(); run_wait_until_tests<DeterministicAtomic>();
} }
TEST(Futex, wake_blocked_live) {
run_wake_blocked_test<std::atomic>();
}
TEST(Futex, wake_blocked_emulated) {
run_wake_blocked_test<EmulatedFutexAtomic>();
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment