Make Monitor::Wait more robust to spurious Inflate failures.
Bug: 17062710
Change-Id: Ife5f6b335caacc70cab543cd568676d277d3beb6
(cherry picked from commit 6f22fc166ed6c11cad229bff442c064e704de101)
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 @@
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 @@
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 @@
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;
}
- break;
- }
- case LockWord::kFatLocked:
- break; // Already set for a wait.
- default: {
- LOG(FATAL) << "Invalid monitor state " << lock_word.GetState();
- return;
+ case LockWord::kFatLocked: // Unreachable given the loop condition above. Fall-through.
+ 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 @@
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 @@
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_);