diff --git a/art/compiler/dex/mir_graph.cc b/art/compiler/dex/mir_graph.cc index f1de596030..9e7f2e3ae6 100644 --- a/art/compiler/dex/mir_graph.cc +++ b/art/compiler/dex/mir_graph.cc @@ -1251,7 +1251,7 @@ void MIRGraph::DisassembleExtendedInstr(const MIR* mir, std::string* decoded_mir int uses = (ssa_rep != nullptr) ? ssa_rep->num_uses : 0; if (opcode < kMirOpFirst) { - return; // It is not an extended instruction. + return; // It is not an extended instruction. } decoded_mir->append(extended_mir_op_names_[opcode - kMirOpFirst]); diff --git a/art/runtime/base/mutex.cc b/art/runtime/base/mutex.cc index 45acc839f5..246b94f045 100644 --- a/art/runtime/base/mutex.cc +++ b/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); diff --git a/art/runtime/base/mutex.h b/art/runtime/base/mutex.h index 62e4e8b48d..26312cd7db 100644 --- a/art/runtime/base/mutex.h +++ b/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_); diff --git a/art/runtime/runtime.cc b/art/runtime/runtime.cc index 17b2bfb784..a3ae59a5da 100644 --- a/art/runtime/runtime.cc +++ b/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); diff --git a/art/runtime/runtime.h b/art/runtime/runtime.h index bacb8e98b7..160e76c278 100644 --- a/art/runtime/runtime.h +++ b/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& GetCompileTimeClassPath(jobject class_loader); void SetCompileTimeClassPath(jobject class_loader, std::vector& 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 method_verifiers_; + std::set method_verifiers_ GUARDED_BY(Locks::method_verifiers_lock_); // 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. diff --git a/art/runtime/verifier/reg_type_cache.cc b/art/runtime/verifier/reg_type_cache.cc index 4bb99c1742..9d2500b0ff 100644 --- a/art/runtime/verifier/reg_type_cache.cc +++ b/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) : can_load_classes_(can_load_classes) { +RegTypeCache::RegTypeCache(bool can_load_classes): entries_lock_("entries lock"), + 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); } diff --git a/art/runtime/verifier/reg_type_cache.h b/art/runtime/verifier/reg_type_cache.h index a75aaaba9c..0adf4a87d8 100644 --- a/art/runtime/verifier/reg_type_cache.h +++ b/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 entries_;