Fix moving GC bugs in MonitorEnter and MonitorExit

Fixes test 088 with gcstress mode.

Change-Id: Iaeb91f62f22233e403e97e954bfdc8dc367e63c8

5 files changed, 35 insertions, 15 deletions

diff --git a/runtime/entrypoints/quick/quick_lock_entrypoints.cc b/runtime/entrypoints/quick/quick_lock_entrypoints.cc
index 3bf001e249..4adb39b9c6 100644
--- a/runtime/entrypoints/quick/quick_lock_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_lock_entrypoints.cc
@@ -21,8 +21,9 @@
 namespace art {
 
 extern "C" int artLockObjectFromCode(mirror::Object* obj, Thread* self)
-    SHARED_REQUIRES(Locks::mutator_lock_)
-    NO_THREAD_SAFETY_ANALYSIS /* EXCLUSIVE_LOCK_FUNCTION(Monitor::monitor_lock_) */ {
+    NO_THREAD_SAFETY_ANALYSIS
+    REQUIRES(!Roles::uninterruptible_)
+    SHARED_REQUIRES(Locks::mutator_lock_) /* EXCLUSIVE_LOCK_FUNCTION(Monitor::monitor_lock_) */ {
   ScopedQuickEntrypointChecks sqec(self);
   if (UNLIKELY(obj == nullptr)) {
     ThrowNullPointerException("Null reference used for synchronization (monitor-enter)");
@@ -41,8 +42,9 @@ extern "C" int artLockObjectFromCode(mirror::Object* obj, Thread* self)
 }
 
 extern "C" int artUnlockObjectFromCode(mirror::Object* obj, Thread* self)
-    SHARED_REQUIRES(Locks::mutator_lock_)
-    NO_THREAD_SAFETY_ANALYSIS /* UNLOCK_FUNCTION(Monitor::monitor_lock_) */ {
+    NO_THREAD_SAFETY_ANALYSIS
+    REQUIRES(!Roles::uninterruptible_)
+    SHARED_REQUIRES(Locks::mutator_lock_) /* UNLOCK_FUNCTION(Monitor::monitor_lock_) */ {
   ScopedQuickEntrypointChecks sqec(self);
   if (UNLIKELY(obj == nullptr)) {
     ThrowNullPointerException("Null reference used for synchronization (monitor-exit)");
diff --git a/runtime/interpreter/interpreter_common.h b/runtime/interpreter/interpreter_common.h
index a5a8d811cb..8c495fc7bb 100644
--- a/runtime/interpreter/interpreter_common.h
+++ b/runtime/interpreter/interpreter_common.h
@@ -83,17 +83,25 @@ void ThrowNullPointerExceptionFromInterpreter()
 template <bool kMonitorCounting>
 static inline void DoMonitorEnter(Thread* self,
                                   ShadowFrame* frame,
-                                  Object* ref) NO_THREAD_SAFETY_ANALYSIS {
-  ref->MonitorEnter(self);
-  frame->GetLockCountData().AddMonitor<kMonitorCounting>(self, ref);
+                                  Object* ref)
+    NO_THREAD_SAFETY_ANALYSIS
+    REQUIRES(!Roles::uninterruptible_) {
+  StackHandleScope<1> hs(self);
+  Handle<Object> h_ref(hs.NewHandle(ref));
+  h_ref->MonitorEnter(self);
+  frame->GetLockCountData().AddMonitor<kMonitorCounting>(self, h_ref.Get());
 }
 
 template <bool kMonitorCounting>
 static inline void DoMonitorExit(Thread* self,
                                  ShadowFrame* frame,
-                                 Object* ref) NO_THREAD_SAFETY_ANALYSIS {
-  ref->MonitorExit(self);
-  frame->GetLockCountData().RemoveMonitorOrThrow<kMonitorCounting>(self, ref);
+                                 Object* ref)
+    NO_THREAD_SAFETY_ANALYSIS
+    REQUIRES(!Roles::uninterruptible_) {
+  StackHandleScope<1> hs(self);
+  Handle<Object> h_ref(hs.NewHandle(ref));
+  h_ref->MonitorExit(self);
+  frame->GetLockCountData().RemoveMonitorOrThrow<kMonitorCounting>(self, h_ref.Get());
 }
 
 void AbortTransactionF(Thread* self, const char* fmt, ...)
diff --git a/runtime/mirror/object.h b/runtime/mirror/object.h
index 50490bbcae..f75b8aeef4 100644
--- a/runtime/mirror/object.h
+++ b/runtime/mirror/object.h
@@ -137,9 +137,13 @@ class MANAGED LOCKABLE Object {
       SHARED_REQUIRES(Locks::mutator_lock_);
   uint32_t GetLockOwnerThreadId();
 
-  mirror::Object* MonitorEnter(Thread* self) SHARED_REQUIRES(Locks::mutator_lock_)
-      EXCLUSIVE_LOCK_FUNCTION();
-  bool MonitorExit(Thread* self) SHARED_REQUIRES(Locks::mutator_lock_)
+  mirror::Object* MonitorEnter(Thread* self)
+      EXCLUSIVE_LOCK_FUNCTION()
+      REQUIRES(!Roles::uninterruptible_)
+      SHARED_REQUIRES(Locks::mutator_lock_);
+  bool MonitorExit(Thread* self)
+      REQUIRES(!Roles::uninterruptible_)
+      SHARED_REQUIRES(Locks::mutator_lock_)
       UNLOCK_FUNCTION();
   void Notify(Thread* self) SHARED_REQUIRES(Locks::mutator_lock_);
   void NotifyAll(Thread* self) SHARED_REQUIRES(Locks::mutator_lock_);
diff --git a/runtime/monitor.cc b/runtime/monitor.cc
index fa5841882e..255a0f23d8 100644
--- a/runtime/monitor.cc
+++ b/runtime/monitor.cc
@@ -696,6 +696,7 @@ static mirror::Object* FakeUnlock(mirror::Object* obj)
 mirror::Object* Monitor::MonitorEnter(Thread* self, mirror::Object* obj) {
   DCHECK(self != nullptr);
   DCHECK(obj != nullptr);
+  self->AssertThreadSuspensionIsAllowable();
   obj = FakeLock(obj);
   uint32_t thread_id = self->GetThreadId();
   size_t contention_count = 0;
@@ -771,6 +772,7 @@ mirror::Object* Monitor::MonitorEnter(Thread* self, mirror::Object* obj) {
 bool Monitor::MonitorExit(Thread* self, mirror::Object* obj) {
   DCHECK(self != nullptr);
   DCHECK(obj != nullptr);
+  self->AssertThreadSuspensionIsAllowable();
   obj = FakeUnlock(obj);
   StackHandleScope<1> hs(self);
   Handle<mirror::Object> h_obj(hs.NewHandle(obj));
diff --git a/runtime/monitor.h b/runtime/monitor.h
index 8cd93c69d7..61235efd89 100644
--- a/runtime/monitor.h
+++ b/runtime/monitor.h
@@ -65,12 +65,16 @@ class Monitor {
   // NO_THREAD_SAFETY_ANALYSIS for mon->Lock.
   static mirror::Object* MonitorEnter(Thread* thread, mirror::Object* obj)
       EXCLUSIVE_LOCK_FUNCTION(obj)
-      SHARED_REQUIRES(Locks::mutator_lock_) NO_THREAD_SAFETY_ANALYSIS;
+      NO_THREAD_SAFETY_ANALYSIS
+      REQUIRES(!Roles::uninterruptible_)
+      SHARED_REQUIRES(Locks::mutator_lock_);
 
   // NO_THREAD_SAFETY_ANALYSIS for mon->Unlock.
   static bool MonitorExit(Thread* thread, mirror::Object* obj)
+      NO_THREAD_SAFETY_ANALYSIS
+      REQUIRES(!Roles::uninterruptible_)
       SHARED_REQUIRES(Locks::mutator_lock_)
-      UNLOCK_FUNCTION(obj) NO_THREAD_SAFETY_ANALYSIS;
+      UNLOCK_FUNCTION(obj);
 
   static void Notify(Thread* self, mirror::Object* obj) SHARED_REQUIRES(Locks::mutator_lock_) {
     DoNotify(self, obj, false);