From 0cfccb799ce68cc66d389e6885f8d73f95ee5c4f Mon Sep 17 00:00:00 2001
From: Steve Fung <stevefung@google.com>
Date: Tue, 19 Jul 2016 17:27:35 -0700
Subject: 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
---
 .../persistent_memory_allocator_unittest.cc        | 29 ++++++++++++++++------
 1 file changed, 21 insertions(+), 8 deletions(-)

(limited to 'base')

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();
 
-- 
cgit v1.2.3