commit a6e7f0872c42009ecbee82d7fbe452deef9ae65b
author Mathieu Chartier <mathieuc@google.com> Thu May 22 14:43:37 2014 -0700
committer Mathieu Chartier <mathieuc@google.com> Thu May 22 14:43:37 2014 -0700
tree 1ba6d026f6c9a47b9961c62511ab47c6d69314dd
parent 697726d42eaf804a1124c25dec58c2b0013a30e9

Move SetMonitorEnterObject outside of blocked thread state change.

Race condition:
Thread is suspended in monitor kBlocked, GC decides to run the
checkpoint on it. The GC sees that the object is non null, and goes
to mark it, but then the thread does SetMonitorObject(nullptr).
Which causes a null object to be marked.

Change-Id: Ie8a5074112947ec07d01ccb813ca2c1bb9ac7066

runtime/monitor.cc

diff --git a/runtime/monitor.cc b/runtime/monitor.cc
index cc38c59..4a4dffb 100644
--- a/runtime/monitor.cc
+++ b/runtime/monitor.cc
@@ -226,9 +226,9 @@
     // Do this before releasing the lock so that we don't get deflated.
     ++num_waiters_;
     monitor_lock_.Unlock(self);  // Let go of locks in order.
+    self->SetMonitorEnterObject(GetObject());
     {
       ScopedThreadStateChange tsc(self, kBlocked);  // Change to blocked and give up mutator_lock_.
-      self->SetMonitorEnterObject(GetObject());
       MutexLock mu2(self, monitor_lock_);  // Reacquire monitor_lock_ without mutator_lock_ for Wait.
       if (owner_ != NULL) {  // Did the owner_ give the lock up?
         monitor_contenders_.Wait(self);  // Still contended so wait.
@@ -249,8 +249,8 @@
           }
         }
       }
-      self->SetMonitorEnterObject(nullptr);
     }
+    self->SetMonitorEnterObject(nullptr);
     monitor_lock_.Lock(self);  // Reacquire locks in order.
     --num_waiters_;
   }

runtime/thread.h

diff --git a/runtime/thread.h b/runtime/thread.h
index de054ee..9a7cb48 100644
--- a/runtime/thread.h
+++ b/runtime/thread.h
@@ -396,11 +396,11 @@
   // Convert a jobject into a Object*
   mirror::Object* DecodeJObject(jobject obj) const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
-  mirror::Object* GetMonitorEnterObject() const {
+  mirror::Object* GetMonitorEnterObject() const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
     return tlsPtr_.monitor_enter_object;
   }
 
-  void SetMonitorEnterObject(mirror::Object* obj) {
+  void SetMonitorEnterObject(mirror::Object* obj) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
     tlsPtr_.monitor_enter_object = obj;
   }