commit 861af894a5d2d4a77eb66041bddfbaf54d8c8ee8
author Alex Light <allight@google.com> Tue Feb 28 12:45:36 2017 -0800
committer Alex Light <allight@google.com> Tue Feb 28 14:35:47 2017 -0800
tree 757339c632345f2e5f5005ba084d2020e7f91548
parent edb792d53fd8b9f94244396a698fb1dc68cb6905

Fix mutex issue

We were having some issues with mutex locking order related to
upgrading local to global references when recording non-debuggable
classes. We refactored this code so there is no longer this issue.

Bug: 35838746
Test: Build and boot a marlin with libartd.so

Change-Id: I093b433d921478307130c49a07d0c7ec34dd070d

runtime/native/dalvik_system_ZygoteHooks.cc

diff --git a/runtime/native/dalvik_system_ZygoteHooks.cc b/runtime/native/dalvik_system_ZygoteHooks.cc
index 100f476..bb92ca7 100644
--- a/runtime/native/dalvik_system_ZygoteHooks.cc
+++ b/runtime/native/dalvik_system_ZygoteHooks.cc
@@ -74,12 +74,35 @@
   }
 }
 
-static void DoCollectNonDebuggableCallback(Thread* thread, void* data ATTRIBUTE_UNUSED)
+class ClassSet {
+ public:
+  explicit ClassSet(Thread* const self) : hs_(self) {}
+
+  void AddClass(ObjPtr<mirror::Class> klass) REQUIRES(Locks::mutator_lock_) {
+    for (Handle<mirror::Class> k : class_set_) {
+      if (k.Get() == klass.Ptr()) {
+        return;
+      }
+    }
+    class_set_.push_back(hs_.NewHandle<mirror::Class>(klass));
+  }
+
+  const std::vector<Handle<mirror::Class>>& GetClasses() const {
+    return class_set_;
+  }
+
+ private:
+  VariableSizedHandleScope hs_;
+  std::vector<Handle<mirror::Class>> class_set_;
+};
+
+static void DoCollectNonDebuggableCallback(Thread* thread, void* data)
     REQUIRES(Locks::mutator_lock_) {
   class NonDebuggableStacksVisitor : public StackVisitor {
    public:
-    explicit NonDebuggableStacksVisitor(Thread* t)
-        : StackVisitor(t, nullptr, StackVisitor::StackWalkKind::kIncludeInlinedFrames) {}
+    NonDebuggableStacksVisitor(Thread* t, ClassSet* class_set)
+        : StackVisitor(t, nullptr, StackVisitor::StackWalkKind::kIncludeInlinedFrames),
+          class_set_(class_set) {}
 
     ~NonDebuggableStacksVisitor() OVERRIDE {}
 
@@ -87,7 +110,7 @@
       if (GetMethod()->IsRuntimeMethod()) {
         return true;
       }
-      NonDebuggableClasses::AddNonDebuggableClass(GetMethod()->GetDeclaringClass());
+      class_set_->AddClass(GetMethod()->GetDeclaringClass());
       if (kIsDebugBuild) {
         LOG(INFO) << GetMethod()->GetDeclaringClass()->PrettyClass()
                   << " might not be fully debuggable/deoptimizable due to "
@@ -95,16 +118,36 @@
       }
       return true;
     }
+
+   private:
+    ClassSet* class_set_;
   };
-  NonDebuggableStacksVisitor visitor(thread);
+  NonDebuggableStacksVisitor visitor(thread, reinterpret_cast<ClassSet*>(data));
   visitor.WalkStack();
 }
 
-static void CollectNonDebuggableClasses() {
+static void CollectNonDebuggableClasses() REQUIRES(!Locks::mutator_lock_) {
   Runtime* const runtime = Runtime::Current();
-  ScopedSuspendAll suspend("Checking stacks for non-obsoletable methods!", /*long_suspend*/false);
-  MutexLock mu(Thread::Current(), *Locks::thread_list_lock_);
-  runtime->GetThreadList()->ForEach(DoCollectNonDebuggableCallback, nullptr);
+  Thread* const self = Thread::Current();
+  // Get the mutator lock.
+  ScopedObjectAccess soa(self);
+  ClassSet classes(self);
+  {
+    // Drop the mutator lock.
+    self->TransitionFromRunnableToSuspended(art::ThreadState::kNative);
+    {
+      // Get it back with a suspend all.
+      ScopedSuspendAll suspend("Checking stacks for non-obsoletable methods!",
+                               /*long_suspend*/false);
+      MutexLock mu(Thread::Current(), *Locks::thread_list_lock_);
+      runtime->GetThreadList()->ForEach(DoCollectNonDebuggableCallback, &classes);
+    }
+    // Recover the shared lock before we leave this scope.
+    self->TransitionFromSuspendedToRunnable();
+  }
+  for (Handle<mirror::Class> klass : classes.GetClasses()) {
+    NonDebuggableClasses::AddNonDebuggableClass(klass.Get());
+  }
 }
 
 static void EnableDebugFeatures(uint32_t debug_flags) {

runtime/non_debuggable_classes.h

diff --git a/runtime/non_debuggable_classes.h b/runtime/non_debuggable_classes.h
index b72afd8..0c94dc0 100644
--- a/runtime/non_debuggable_classes.h
+++ b/runtime/non_debuggable_classes.h
@@ -35,7 +35,8 @@
     return non_debuggable_classes;
   }
 
-  static void AddNonDebuggableClass(ObjPtr<mirror::Class> klass) REQUIRES(Locks::mutator_lock_);
+  static void AddNonDebuggableClass(ObjPtr<mirror::Class> klass)
+      REQUIRES_SHARED(Locks::mutator_lock_);
 
  private:
   static std::vector<jclass> non_debuggable_classes;