commit aa5168291c46f9b418d989bccf2d8e09338a83e6
author Mathieu Chartier <mathieuc@google.com> Fri Oct 02 15:53:37 2015 -0700
committer Mathieu Chartier <mathieuc@google.com> Mon Oct 05 11:50:18 2015 -0700
tree bad4c44f503962fc5d74c3079f14f9af1ce7864d
parent 1aba494df441c31836e5b7d4d8e408420a93cb8d

Add exclusion between instrumentation and GC

Instrumentation calls VisitClasses while exclusive holding the
mutator lock. This can cause deadlocks since VisitClasses needs to
decode JNI weak globals. If the system weak slow path flag is set,
then we wait holding locks while exclusive holding the mutator lock.
This causes a deadlock since the GC cannot acquire the mutator lock
to sweep system weaks.

This fixes a deadlock seen in one of the tracing tests.

Change-Id: I580152118e068a70f309dcc19df4144afec835dd

runtime/debugger.cc

diff --git a/runtime/debugger.cc b/runtime/debugger.cc
index d24b4fb..b19381d 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -30,6 +30,7 @@
 #include "dex_instruction.h"
 #include "gc/accounting/card_table-inl.h"
 #include "gc/allocation_record.h"
+#include "gc/scoped_gc_critical_section.h"
 #include "gc/space/large_object_space.h"
 #include "gc/space/space-inl.h"
 #include "handle_scope.h"
@@ -559,14 +560,15 @@
     return;
   }
 
+  Thread* const self = Thread::Current();
   {
     // TODO: dalvik only warned if there were breakpoints left over. clear in Dbg::Disconnected?
-    ReaderMutexLock mu(Thread::Current(), *Locks::breakpoint_lock_);
+    ReaderMutexLock mu(self, *Locks::breakpoint_lock_);
     CHECK_EQ(gBreakpoints.size(), 0U);
   }
 
   {
-    MutexLock mu(Thread::Current(), *Locks::deoptimization_lock_);
+    MutexLock mu(self, *Locks::deoptimization_lock_);
     CHECK_EQ(deoptimization_requests_.size(), 0U);
     CHECK_EQ(full_deoptimization_event_count_, 0U);
     CHECK_EQ(dex_pc_change_event_ref_count_, 0U);
@@ -598,6 +600,10 @@
   Runtime* runtime = Runtime::Current();
   Thread* self = Thread::Current();
   {
+    // Required for DisableDeoptimization.
+    gc::ScopedGCCriticalSection gcs(self,
+                                    gc::kGcCauseInstrumentation,
+                                    gc::kCollectorTypeInstrumentation);
     ScopedSuspendAll ssa(__FUNCTION__);
     ThreadState old_state = self->SetStateUnsafe(kRunnable);
     // Debugger may not be active at this point.
@@ -3162,6 +3168,10 @@
   }
   CHECK_EQ(self->GetState(), kRunnable);
   ScopedThreadSuspension sts(self, kWaitingForDeoptimization);
+  // Required for ProcessDeoptimizationRequest.
+  gc::ScopedGCCriticalSection gcs(self,
+                                  gc::kGcCauseInstrumentation,
+                                  gc::kCollectorTypeInstrumentation);
   // We need to suspend mutator threads first.
   ScopedSuspendAll ssa(__FUNCTION__);
   const ThreadState old_state = self->SetStateUnsafe(kRunnable);