[art] ART: Add entries_lock for race condition

GCDaemon thread would visit incorrect RegType content when there is
another thread initializing classes.
Add a lock to protect entries_.

https://code.google.com/p/android/issues/detail?id=159849

Change-Id: Iabaa1c7f5cc5106b60a6e3856152e0797e8a5d6d

art/runtime/base/mutex.cc

@@ -41,6 +41,7 @@ Mutex* Locks::instrument_entrypoints_lock_ = nullptr;
 Mutex* Locks::intern_table_lock_ = nullptr;
 Mutex* Locks::logging_lock_ = nullptr;
 Mutex* Locks::mem_maps_lock_ = nullptr;
+Mutex* Locks::method_verifiers_lock_ = nullptr;
 Mutex* Locks::modify_ldt_lock_ = nullptr;
 ReaderWriterMutex* Locks::mutator_lock_ = nullptr;
 Mutex* Locks::profiler_lock_ = nullptr;
@@ -922,6 +923,10 @@ void Locks::Init() {
     classlinker_classes_lock_ = new ReaderWriterMutex("ClassLinker classes lock",
                                                       current_lock_level);
 
+    UPDATE_CURRENT_LOCK_LEVEL(kMethodVerifiersLock);
+    DCHECK(method_verifiers_lock_ == nullptr);
+    method_verifiers_lock_ = new Mutex("Method verifiers lock", current_lock_level);
+
     UPDATE_CURRENT_LOCK_LEVEL(kMonitorPoolLock);
     DCHECK(allocated_monitor_ids_lock_ == nullptr);
     allocated_monitor_ids_lock_ =  new Mutex("allocated monitor ids lock", current_lock_level);

art/runtime/base/mutex.h

@@ -84,6 +84,7 @@ enum LockLevel {
   kModifyLdtLock,
   kAllocatedThreadIdsLock,
   kMonitorPoolLock,
+  kMethodVerifiersLock,
   kClassLinkerClassesLock,
   kBreakpointLock,
   kMonitorLock,
@@ -575,9 +576,11 @@ class Locks {
   // Guards lists of classes within the class linker.
   static ReaderWriterMutex* classlinker_classes_lock_ ACQUIRED_AFTER(breakpoint_lock_);
 
+  static Mutex* method_verifiers_lock_ ACQUIRED_AFTER(classlinker_classes_lock_);
+
   // When declaring any Mutex add DEFAULT_MUTEX_ACQUIRED_AFTER to use annotalysis to check the code
   // doesn't try to hold a higher level Mutex.
-  #define DEFAULT_MUTEX_ACQUIRED_AFTER ACQUIRED_AFTER(Locks::classlinker_classes_lock_)
+  #define DEFAULT_MUTEX_ACQUIRED_AFTER ACQUIRED_AFTER(Locks::method_verifiers_lock_)
 
   static Mutex* allocated_monitor_ids_lock_ ACQUIRED_AFTER(classlinker_classes_lock_);
 

art/runtime/runtime.cc

@@ -118,7 +118,6 @@ Runtime::Runtime()
       java_vm_(nullptr),
       fault_message_lock_("Fault message lock"),
       fault_message_(""),
-      method_verifier_lock_("Method verifiers lock"),
       threads_being_born_(0),
       shutdown_cond_(new ConditionVariable("Runtime shutdown", *Locks::runtime_shutdown_lock_)),
       shutting_down_(false),
@@ -1171,7 +1170,7 @@ void Runtime::VisitNonThreadRoots(RootCallback* callback, void* arg) {
   }
   verifier::MethodVerifier::VisitStaticRoots(callback, arg);
   {
-    MutexLock mu(Thread::Current(), method_verifier_lock_);
+    MutexLock mu(Thread::Current(), *Locks::method_verifiers_lock_);
     for (verifier::MethodVerifier* verifier : method_verifiers_) {
       verifier->VisitRoots(callback, arg);
     }
@@ -1337,13 +1336,13 @@ void Runtime::SetCompileTimeClassPath(jobject class_loader,
 
 void Runtime::AddMethodVerifier(verifier::MethodVerifier* verifier) {
   DCHECK(verifier != nullptr);
-  MutexLock mu(Thread::Current(), method_verifier_lock_);
+  MutexLock mu(Thread::Current(), *Locks::method_verifiers_lock_);
   method_verifiers_.insert(verifier);
 }
 
 void Runtime::RemoveMethodVerifier(verifier::MethodVerifier* verifier) {
   DCHECK(verifier != nullptr);
-  MutexLock mu(Thread::Current(), method_verifier_lock_);
+  MutexLock mu(Thread::Current(), *Locks::method_verifiers_lock_);
   auto it = method_verifiers_.find(verifier);
   CHECK(it != method_verifiers_.end());
   method_verifiers_.erase(it);

art/runtime/runtime.h

@@ -423,9 +423,10 @@ class Runtime {
     return use_compile_time_class_path_;
   }
 
-  void AddMethodVerifier(verifier::MethodVerifier* verifier) LOCKS_EXCLUDED(method_verifier_lock_);
+  void AddMethodVerifier(verifier::MethodVerifier* verifier)
+      LOCKS_EXCLUDED(Locks::method_verifiers_lock_);
   void RemoveMethodVerifier(verifier::MethodVerifier* verifier)
-      LOCKS_EXCLUDED(method_verifier_lock_);
+      LOCKS_EXCLUDED(Locks::method_verifiers_lock_);
 
   const std::vector<const DexFile*>& GetCompileTimeClassPath(jobject class_loader);
   void SetCompileTimeClassPath(jobject class_loader, std::vector<const DexFile*>& class_path);
@@ -584,8 +585,7 @@ class Runtime {
   std::string fault_message_ GUARDED_BY(fault_message_lock_);
 
   // Method verifier set, used so that we can update their GC roots.
-  Mutex method_verifier_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER;
+  std::set<verifier::MethodVerifier*> method_verifiers_ GUARDED_BY(Locks::method_verifiers_lock_);
-  std::set<verifier::MethodVerifier*> method_verifiers_;
 
   // A non-zero value indicates that a thread has been created but not yet initialized. Guarded by
   // the shutdown lock so that threads aren't born while we're shutting down.

art/runtime/verifier/reg_type_cache.cc

@@ -235,8 +235,8 @@ RegType& RegTypeCache::FromClass(const char* descriptor, mirror::Class* klass, b
     return *entry;
   }
 }
-
+RegTypeCache::RegTypeCache(bool can_load_classes): entries_lock_("entries lock"),
-RegTypeCache::RegTypeCache(bool can_load_classes) : can_load_classes_(can_load_classes) {
+                can_load_classes_(can_load_classes) {
   if (kIsDebugBuild && can_load_classes) {
     Thread::Current()->AssertThreadSuspensionIsAllowable();
   }
@@ -611,12 +611,15 @@ void RegTypeCache::VisitStaticRoots(RootCallback* callback, void* arg) {
 }
 
 void RegTypeCache::VisitRoots(RootCallback* callback, void* arg) {
+  MutexLock mu(Thread::Current(), entries_lock_);
   for (RegType* entry : entries_) {
     entry->VisitRoots(callback, arg);
   }
 }
 
 void RegTypeCache::AddEntry(RegType* new_entry) {
+  // TODO: There is probably a faster way to do this by using thread local roots.
+  MutexLock mu(Thread::Current(), entries_lock_);
   entries_.push_back(new_entry);
 }
 

art/runtime/verifier/reg_type_cache.h

@@ -165,6 +165,9 @@ class RegTypeCache {
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
   static void CreatePrimitiveAndSmallConstantTypes() SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
+  // Guards adding and visitng roots to prevent race conditions.
+  Mutex entries_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER;
+
   // The actual storage for the RegTypes.
   std::vector<RegType*> entries_;