commit f8349361a16a4e2796efe9f3586b994e8d4834e4
author Elliott Hughes <enh@google.com> Mon Jun 18 15:00:06 2012 -0700
committer Elliott Hughes <enh@google.com> Mon Jun 18 15:16:21 2012 -0700
tree 0a3474cde4028e1915c59393f49c147dd76ad766
parent 44335e189951a863607049a33571932fb6a2a841

Use static thread safety analysis when available, and fix the bugs GCC finds.

It's impossible to express the Heap locking and the ThreadList locking with
GCC, but Clang is supposed to be able to do it. This patch does what's possible
for now.

Change-Id: Ib64a890c9d27c6ce255d5003cb755c2ef1beba95

src/class_linker.cc

diff --git a/src/class_linker.cc b/src/class_linker.cc
index a692e2c..47beadb 100644
--- a/src/class_linker.cc
+++ b/src/class_linker.cc
@@ -658,6 +658,7 @@
 }
 
 const OatFile* ClassLinker::FindOpenedOatFileFromDexLocation(const std::string& dex_location) {
+  MutexLock mu(dex_lock_);
   for (size_t i = 0; i < oat_files_.size(); i++) {
     const OatFile* oat_file = oat_files_[i];
     DCHECK(oat_file != NULL);
@@ -807,6 +808,7 @@
 }
 
 const OatFile* ClassLinker::FindOpenedOatFileFromOatLocation(const std::string& oat_location) {
+  MutexLock mu(dex_lock_);
   for (size_t i = 0; i < oat_files_.size(); i++) {
     const OatFile* oat_file = oat_files_[i];
     DCHECK(oat_file != NULL);
@@ -928,8 +930,11 @@
 void ClassLinker::VisitRoots(Heap::RootVisitor* visitor, void* arg) const {
   visitor(class_roots_, arg);
 
-  for (size_t i = 0; i < dex_caches_.size(); i++) {
-    visitor(dex_caches_[i], arg);
+  {
+    MutexLock mu(dex_lock_);
+    for (size_t i = 0; i < dex_caches_.size(); i++) {
+      visitor(dex_caches_[i], arg);
+    }
   }
 
   {
@@ -1831,11 +1836,11 @@
   size_t hash = StringPieceHash()(descriptor);
   MutexLock mu(classes_lock_);
   Table& classes = image_class ? image_classes_ : classes_;
-  Class* existing = LookupClass(descriptor.data(), klass->GetClassLoader(), hash, classes);
+  Class* existing = LookupClassLocked(descriptor.data(), klass->GetClassLoader(), hash, classes);
 #ifndef NDEBUG
   // Check we don't have the class in the other table in error
   Table& other_classes = image_class ? classes_ : image_classes_;
-  CHECK(LookupClass(descriptor.data(), klass->GetClassLoader(), hash, other_classes) == NULL);
+  CHECK(LookupClassLocked(descriptor.data(), klass->GetClassLoader(), hash, other_classes) == NULL);
 #endif
   if (existing != NULL) {
     return existing;
@@ -1873,15 +1878,15 @@
   size_t hash = Hash(descriptor);
   MutexLock mu(classes_lock_);
   // TODO: determine if its better to search classes_ or image_classes_ first
-  Class* klass = LookupClass(descriptor, class_loader, hash, classes_);
+  Class* klass = LookupClassLocked(descriptor, class_loader, hash, classes_);
   if (klass != NULL) {
     return klass;
   }
-  return LookupClass(descriptor, class_loader, hash, image_classes_);
+  return LookupClassLocked(descriptor, class_loader, hash, image_classes_);
 }
 
-Class* ClassLinker::LookupClass(const char* descriptor, const ClassLoader* class_loader,
-                                size_t hash, const Table& classes) {
+Class* ClassLinker::LookupClassLocked(const char* descriptor, const ClassLoader* class_loader,
+                                      size_t hash, const Table& classes) {
   ClassHelper kh(NULL, this);
   typedef Table::const_iterator It;  // TODO: C++0x auto
   for (It it = classes.lower_bound(hash), end = classes_.end(); it != end && it->first == hash; ++it) {
@@ -3513,6 +3518,7 @@
 }
 
 void ClassLinker::RelocateExecutable() {
+  MutexLock mu(dex_lock_);
   for (size_t i = 0; i < oat_files_.size(); ++i) {
     const_cast<OatFile*>(oat_files_[i])->RelocateExecutable();
   }