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
diff --git a/runtime/base/mutex.cc b/runtime/base/mutex.cc
index 13dcb8c..2ec2b0c 100644
--- a/runtime/base/mutex.cc
+++ b/runtime/base/mutex.cc
@@ -46,6 +46,7 @@
Mutex* Locks::jni_libraries_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;
@@ -1001,6 +1002,10 @@
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/runtime/base/mutex.h b/runtime/base/mutex.h
index 3b052c0..f9e1e62 100644
--- a/runtime/base/mutex.h
+++ b/runtime/base/mutex.h
@@ -86,6 +86,7 @@
kModifyLdtLock,
kAllocatedThreadIdsLock,
kMonitorPoolLock,
+ kMethodVerifiersLock,
kClassLinkerClassesLock,
kBreakpointLock,
kMonitorLock,
@@ -587,9 +588,11 @@
// 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/runtime/runtime.cc b/runtime/runtime.cc
index 0728646..a2f1481 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -148,7 +148,6 @@
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),
@@ -1333,7 +1332,7 @@
}
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);
}
@@ -1514,7 +1513,7 @@
if (gAborting) {
return;
}
- MutexLock mu(Thread::Current(), method_verifier_lock_);
+ MutexLock mu(Thread::Current(), *Locks::method_verifiers_lock_);
method_verifiers_.insert(verifier);
}
@@ -1523,7 +1522,7 @@
if (gAborting) {
return;
}
- 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/runtime/runtime.h b/runtime/runtime.h
index 4cddb5c..d54972c 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -456,9 +456,10 @@
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);
@@ -642,8 +643,7 @@
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_;
+ std::set<verifier::MethodVerifier*> 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/runtime/verifier/reg_type_cache.cc b/runtime/verifier/reg_type_cache.cc
index c248565..22696c7 100644
--- a/runtime/verifier/reg_type_cache.cc
+++ b/runtime/verifier/reg_type_cache.cc
@@ -238,7 +238,9 @@
}
}
-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) {
Thread::Current()->AssertThreadSuspensionIsAllowable(gAborting == 0);
}
@@ -580,6 +582,7 @@
}
void RegTypeCache::VisitRoots(RootCallback* callback, void* arg) {
+ MutexLock mu(Thread::Current(), entries_lock_);
// Exclude the static roots that are visited by VisitStaticRoots().
for (size_t i = primitive_count_; i < entries_.size(); ++i) {
entries_[i]->VisitRoots(callback, arg);
@@ -587,6 +590,8 @@
}
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/runtime/verifier/reg_type_cache.h b/runtime/verifier/reg_type_cache.h
index ff7b1f3..4b56fd6 100644
--- a/runtime/verifier/reg_type_cache.h
+++ b/runtime/verifier/reg_type_cache.h
@@ -171,6 +171,9 @@
// Number of well known primitives that will be copied into a RegTypeCache upon construction.
static uint16_t primitive_count_;
+ // Guards adding and visitng roots to prevent race conditions.
+ Mutex entries_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER;
+
// The actual storage for the RegTypes.
std::vector<const RegType*> entries_;