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/trace.cc

diff --git a/runtime/trace.cc b/runtime/trace.cc
index e2743ce..eb0ea68 100644
--- a/runtime/trace.cc
+++ b/runtime/trace.cc
@@ -31,6 +31,7 @@
 #include "common_throws.h"
 #include "debugger.h"
 #include "dex_file-inl.h"
+#include "gc/scoped_gc_critical_section.h"
 #include "instrumentation.h"
 #include "mirror/class-inl.h"
 #include "mirror/dex_cache-inl.h"
@@ -350,6 +351,10 @@
 
   // Create Trace object.
   {
+    // Required since EnableMethodTracing calls ConfigureStubs which visits class linker classes.
+    gc::ScopedGCCriticalSection gcs(self,
+                                    gc::kGcCauseInstrumentation,
+                                    gc::kCollectorTypeInstrumentation);
     ScopedSuspendAll ssa(__FUNCTION__);
     MutexLock mu(self, *Locks::trace_lock_);
     if (the_trace_ != nullptr) {
@@ -464,9 +469,10 @@
   Runtime* runtime = Runtime::Current();
   Trace* the_trace = nullptr;
 
+  Thread* const self = Thread::Current();
   pthread_t sampling_pthread = 0U;
   {
-    MutexLock mu(Thread::Current(), *Locks::trace_lock_);
+    MutexLock mu(self, *Locks::trace_lock_);
     if (the_trace_ == nullptr) {
       LOG(ERROR) << "Trace pause requested, but no trace currently running";
       return;
@@ -478,23 +484,26 @@
 
   if (sampling_pthread != 0U) {
     {
-      MutexLock mu(Thread::Current(), *Locks::trace_lock_);
+      MutexLock mu(self, *Locks::trace_lock_);
       the_trace_ = nullptr;
     }
     CHECK_PTHREAD_CALL(pthread_join, (sampling_pthread, nullptr), "sampling thread shutdown");
     sampling_pthread_ = 0U;
     {
-      MutexLock mu(Thread::Current(), *Locks::trace_lock_);
+      MutexLock mu(self, *Locks::trace_lock_);
       the_trace_ = the_trace;
     }
   }
 
   if (the_trace != nullptr) {
+    gc::ScopedGCCriticalSection gcs(self,
+                                    gc::kGcCauseInstrumentation,
+                                    gc::kCollectorTypeInstrumentation);
     ScopedSuspendAll ssa(__FUNCTION__);
     stop_alloc_counting = (the_trace->flags_ & Trace::kTraceCountAllocs) != 0;
 
     if (the_trace->trace_mode_ == TraceMode::kSampling) {
-      MutexLock mu(Thread::Current(), *Locks::thread_list_lock_);
+      MutexLock mu(self, *Locks::thread_list_lock_);
       runtime->GetThreadList()->ForEach(ClearThreadStackTraceAndClockBase, nullptr);
     } else {
       runtime->GetInstrumentation()->DisableMethodTracing(kTracerInstrumentationKey);
@@ -530,6 +539,9 @@
   bool enable_stats = (the_trace->flags_ && kTraceCountAllocs) != 0;
 
   {
+    gc::ScopedGCCriticalSection gcs(self,
+                                    gc::kGcCauseInstrumentation,
+                                    gc::kCollectorTypeInstrumentation);
     ScopedSuspendAll ssa(__FUNCTION__);
 
     // Reenable.