Make Monitor::Wait more robust to spurious Inflate failures.

Bug: 17062710
Change-Id: Ife5f6b335caacc70cab543cd568676d277d3beb6
(cherry picked from commit 6f22fc166ed6c11cad229bff442c064e704de101)

3 files changed, 32 insertions, 27 deletions

diff --git a/runtime/mirror/object.cc b/runtime/mirror/object.cc
index 3543654..a1177d6 100644
--- a/runtime/mirror/object.cc
+++ b/runtime/mirror/object.cc
@@ -162,7 +162,8 @@ int32_t Object::IdentityHashCode() const {
         break;
       }
       case LockWord::kThinLocked: {
-        // Inflate the thin lock to a monitor and stick the hash code inside of the monitor.
+        // Inflate the thin lock to a monitor and stick the hash code inside of the monitor. May
+        // fail spuriously.
         Thread* self = Thread::Current();
         StackHandleScope<1> hs(self);
         Handle<mirror::Object> h_this(hs.NewHandle(current_this));
diff --git a/runtime/monitor.cc b/runtime/monitor.cc
index 433c1b2..4c780ee 100644
--- a/runtime/monitor.cc
+++ b/runtime/monitor.cc
@@ -641,11 +641,6 @@ bool Monitor::Deflate(Thread* self, mirror::Object* obj) {
   return true;
 }
 
-/*
- * Changes the shape of a monitor from thin to fat, preserving the internal lock state. The calling
- * thread must own the lock or the owner must be suspended. There's a race with other threads
- * inflating the lock and so the caller should read the monitor following the call.
- */
 void Monitor::Inflate(Thread* self, Thread* owner, mirror::Object* obj, int32_t hash_code) {
   DCHECK(self != nullptr);
   DCHECK(obj != nullptr);
@@ -831,30 +826,32 @@ void Monitor::Wait(Thread* self, mirror::Object *obj, int64_t ms, int32_t ns,
   DCHECK(self != nullptr);
   DCHECK(obj != nullptr);
   LockWord lock_word = obj->GetLockWord(true);
-  switch (lock_word.GetState()) {
-    case LockWord::kHashCode:
-      // Fall-through.
-    case LockWord::kUnlocked:
-      ThrowIllegalMonitorStateExceptionF("object not locked by thread before wait()");
-      return;  // Failure.
-    case LockWord::kThinLocked: {
-      uint32_t thread_id = self->GetThreadId();
-      uint32_t owner_thread_id = lock_word.ThinLockOwner();
-      if (owner_thread_id != thread_id) {
+  while (lock_word.GetState() != LockWord::kFatLocked) {
+    switch (lock_word.GetState()) {
+      case LockWord::kHashCode:
+        // Fall-through.
+      case LockWord::kUnlocked:
         ThrowIllegalMonitorStateExceptionF("object not locked by thread before wait()");
         return;  // Failure.
-      } else {
-        // We own the lock, inflate to enqueue ourself on the Monitor.
-        Inflate(self, self, obj, 0);
-        lock_word = obj->GetLockWord(true);
+      case LockWord::kThinLocked: {
+        uint32_t thread_id = self->GetThreadId();
+        uint32_t owner_thread_id = lock_word.ThinLockOwner();
+        if (owner_thread_id != thread_id) {
+          ThrowIllegalMonitorStateExceptionF("object not locked by thread before wait()");
+          return;  // Failure.
+        } else {
+          // We own the lock, inflate to enqueue ourself on the Monitor. May fail spuriously so
+          // re-load.
+          Inflate(self, self, obj, 0);
+          lock_word = obj->GetLockWord(true);
+        }
+        break;
+      }
+      case LockWord::kFatLocked:  // Unreachable given the loop condition above. Fall-through.
+      default: {
+        LOG(FATAL) << "Invalid monitor state " << lock_word.GetState();
+        return;
       }
-      break;
-    }
-    case LockWord::kFatLocked:
-      break;  // Already set for a wait.
-    default: {
-      LOG(FATAL) << "Invalid monitor state " << lock_word.GetState();
-      return;
     }
   }
   Monitor* mon = lock_word.FatLockMonitor();
diff --git a/runtime/monitor.h b/runtime/monitor.h
index 26d43c9..ae14fc1 100644
--- a/runtime/monitor.h
+++ b/runtime/monitor.h
@@ -117,6 +117,7 @@ class Monitor {
     return monitor_id_;
   }
 
+  // Inflate the lock on obj. May fail to inflate for spurious reasons, always re-check.
   static void InflateThinLocked(Thread* self, Handle<mirror::Object> obj, LockWord lock_word,
                                 uint32_t hash_code) NO_THREAD_SAFETY_ANALYSIS;
 
@@ -138,6 +139,12 @@ class Monitor {
   void AppendToWaitSet(Thread* thread) EXCLUSIVE_LOCKS_REQUIRED(monitor_lock_);
   void RemoveFromWaitSet(Thread* thread) EXCLUSIVE_LOCKS_REQUIRED(monitor_lock_);
 
+  /*
+   * Changes the shape of a monitor from thin to fat, preserving the internal lock state. The
+   * calling thread must own the lock or the owner must be suspended. There's a race with other
+   * threads inflating the lock, installing hash codes and spurious failures. The caller should
+   * re-read the lock word following the call.
+   */
   static void Inflate(Thread* self, Thread* owner, mirror::Object* obj, int32_t hash_code)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);