summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSteve Fung <stevefung@google.com>2016-08-03 16:31:19 -0700
committerSteve Fung <stevefung@google.com>2016-08-03 16:31:25 -0700
commit5c7518c7bd1205dab9ba7701c95106f6a6195f3b (patch)
tree489d6a01b62ff33949bd95a2fbd0a8148070cb72
parent7de8dfb2b917082486b21b1666c4687ec05e310b (diff)
downloadlibchrome-5c7518c7bd1205dab9ba7701c95106f6a6195f3b.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=b:30183753 TEST=`base_unittests --gtest_filter=PersistentMemoryAllocatorTest.*` passes Committed: https://crrev.com/ff268f9b1002cedd94e6f6b9b3c6960aeb3037ca Cr-Commit-Position: refs/heads/master@{#409622} Change-Id: I6c5cf36f77655a5b34bce1b005ceda54101febef
-rw-r--r--base/metrics/persistent_memory_allocator_unittest.cc41
1 files changed, 32 insertions, 9 deletions
diff --git a/base/metrics/persistent_memory_allocator_unittest.cc b/base/metrics/persistent_memory_allocator_unittest.cc
index 64333389a1..a3d90c2612 100644
--- a/base/metrics/persistent_memory_allocator_unittest.cc
+++ b/base/metrics/persistent_memory_allocator_unittest.cc
@@ -320,12 +320,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.
@@ -333,8 +335,17 @@ class CounterThread : public SimpleThread {
// releases the next, etc., etc.
{
AutoLock autolock(*lock_);
- condition_->Wait();
- condition_->Signal();
+
+ // Before calling Wait(), make sure that the wake up condition
+ // has not already passed. Also, since spurious signal events
+ // are possible, check the condition in a while loop to make
+ // sure that the wake up condition is met when this thread
+ // returns from the Wait().
+ // See usage comments in src/base/synchronization/condition_variable.h.
+ while (!*wake_up_) {
+ condition_->Wait();
+ condition_->Signal();
+ }
}
uint32_t type;
@@ -350,6 +361,9 @@ class CounterThread : public SimpleThread {
Lock* lock_;
ConditionVariable* condition_;
unsigned count_;
+ bool* wake_up_;
+
+ DISALLOW_COPY_AND_ASSIGN(CounterThread);
};
// Ensure that parallel iteration returns the same number of objects as
@@ -373,12 +387,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();
@@ -386,6 +401,14 @@ TEST_F(PersistentMemoryAllocatorTest, IteratorParallelismTest) {
t4.Start();
t5.Start();
+ // Take the lock and set the wake up condition to true. This helps to
+ // avoid a race condition where the Signal() event is called before
+ // all the threads have reached the Wait() and thus never get woken up.
+ {
+ AutoLock autolock(lock);
+ wake_up = true;
+ }
+
// This will release all the waiting threads.
condition.Signal();