diff options
author | Steve Fung <stevefung@google.com> | 2016-07-19 17:27:35 -0700 |
---|---|---|
committer | Treehugger Robot <treehugger-gerrit@google.com> | 2016-07-20 18:43:17 +0000 |
commit | 0cfccb799ce68cc66d389e6885f8d73f95ee5c4f (patch) | |
tree | 9ffec7dc4d10c0520f30fdbd554124a6f5ac926d /base | |
parent | e0afade07013f884fef7100d290ccf686369d803 (diff) | |
download | libchrome-0cfccb799ce68cc66d389e6885f8d73f95ee5c4f.tar.gz |
Fix a race condition in persistent memory allocator unit test
The PersistentMemoryAllocatorTest.IteratorParallelismTest unit
test gets stuck on a thread's Join() call when run on
brilloemulator-arm. This appears to be due to a race condition
in the thread setting up its lock and waiting on the condition
variable, and the parent thread issuing the Signal() chain that
is meant to wake up the children threads. Add in an autolock in
the main thread and a conditional bool around the threads' call
to Wait() to prevent any threads from calling Wait() after the
main thread has already sent the signal.
Bug: 30183753
Test: `libchrome_test` passes on brilloemulator-arm
Change-Id: Ief955ed8d94c9264e6d3cbbfaad637a1c5fade02
Diffstat (limited to 'base')
-rw-r--r-- | base/metrics/persistent_memory_allocator_unittest.cc | 29 |
1 files changed, 21 insertions, 8 deletions
diff --git a/base/metrics/persistent_memory_allocator_unittest.cc b/base/metrics/persistent_memory_allocator_unittest.cc index 70e13921fa..8a664483c6 100644 --- a/base/metrics/persistent_memory_allocator_unittest.cc +++ b/base/metrics/persistent_memory_allocator_unittest.cc @@ -316,12 +316,14 @@ class CounterThread : public SimpleThread { CounterThread(const std::string& name, PersistentMemoryAllocator::Iterator* iterator, Lock* lock, - ConditionVariable* condition) + ConditionVariable* condition, + bool* wake_up) : SimpleThread(name, Options()), iterator_(iterator), lock_(lock), condition_(condition), - count_(0) {} + count_(0), + wake_up_(wake_up) {} void Run() override { // Wait so all threads can start at approximately the same time. @@ -329,7 +331,9 @@ class CounterThread : public SimpleThread { // releases the next, etc., etc. { AutoLock autolock(*lock_); - condition_->Wait(); + while (!*wake_up_) { + condition_->Wait(); + } condition_->Signal(); } @@ -346,6 +350,7 @@ class CounterThread : public SimpleThread { Lock* lock_; ConditionVariable* condition_; unsigned count_; + bool* wake_up_; }; // Ensure that parallel iteration returns the same number of objects as @@ -369,12 +374,13 @@ TEST_F(PersistentMemoryAllocatorTest, IteratorParallelismTest) { PersistentMemoryAllocator::Iterator iter(allocator_.get()); Lock lock; ConditionVariable condition(&lock); + bool wake_up = false; - CounterThread t1("t1", &iter, &lock, &condition); - CounterThread t2("t2", &iter, &lock, &condition); - CounterThread t3("t3", &iter, &lock, &condition); - CounterThread t4("t4", &iter, &lock, &condition); - CounterThread t5("t5", &iter, &lock, &condition); + CounterThread t1("t1", &iter, &lock, &condition, &wake_up); + CounterThread t2("t2", &iter, &lock, &condition, &wake_up); + CounterThread t3("t3", &iter, &lock, &condition, &wake_up); + CounterThread t4("t4", &iter, &lock, &condition, &wake_up); + CounterThread t5("t5", &iter, &lock, &condition, &wake_up); t1.Start(); t2.Start(); @@ -382,6 +388,13 @@ TEST_F(PersistentMemoryAllocatorTest, IteratorParallelismTest) { t4.Start(); t5.Start(); + // Avoid a race condition with the threads calling Wait() and sending + // the condition.Signal(). + { + AutoLock autolock(lock); + wake_up = true; + } + // This will release all the waiting threads. condition.Signal(); |