commit e46cd75f182a3d738c5e2ef3cc90b2f0b1de56ee
author Mathieu Chartier <mathieuc@google.com> Wed Oct 31 16:56:18 2012 -0700
committer Mathieu Chartier <mathieuc@google.com> Wed Oct 31 16:56:18 2012 -0700
tree ffd976fbc1195cddbb956f87a73d423d611b3cf9
parent 332f6827edf72f9b3d247a6c2fa79e4d520a409d

Fix race condition in thread pool shutdown

We were not holding the task queue lock when we did broadcast, causing
a race where a thread waits on the CV after the broadcast.

Fixes dex2oat deadlocking.

Change-Id: I84f30020511c2bd43f71d9b7b392720bd8d03eab

src/mutex.cc

diff --git a/src/mutex.cc b/src/mutex.cc
index a4347ba..0e33f3c 100644
--- a/src/mutex.cc
+++ b/src/mutex.cc
@@ -749,6 +749,7 @@
   DCHECK(self == NULL || self == Thread::Current());
   // TODO: enable below, there's a race in thread creation that causes false failures currently.
   // guard_.AssertExclusiveHeld(self);
+  DCHECK_EQ(guard_.GetExclusiveOwnerTid(), SafeGetTid(self));
 #if ART_USE_FUTEXES
   if (num_waiters_ > 0) {
     android_atomic_inc(&state_); // Indicate a wake has occurred to waiters coming in.

src/thread_pool.cc

diff --git a/src/thread_pool.cc b/src/thread_pool.cc
index fa0cf79..f3319e2 100644
--- a/src/thread_pool.cc
+++ b/src/thread_pool.cc
@@ -69,11 +69,15 @@
 }
 
 ThreadPool::~ThreadPool() {
-  // Tell any remaining workers to shut down.
-  shutting_down_ = true;
-  android_memory_barrier();
-  // Broadcast to everyone waiting.
-  task_queue_condition_.Broadcast(Thread::Current());
+  {
+    Thread* self = Thread::Current();
+    MutexLock mu(self, task_queue_lock_);
+    // Tell any remaining workers to shut down.
+    shutting_down_ = true;
+    android_memory_barrier();
+    // Broadcast to everyone waiting.
+    task_queue_condition_.Broadcast(self);
+  }
   // Wait for the threads to finish.
   STLDeleteElements(&threads_);
 }