5 files changed, 171 insertions, 58 deletions

diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 5f2c944838..73da2cbe5b 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -1295,7 +1295,8 @@ bool ClassLinker::ClassInClassTable(mirror::Class* klass) {
 }
 
 void ClassLinker::VisitClassRoots(RootVisitor* visitor, VisitRootFlags flags) {
-  WriterMutexLock mu(Thread::Current(), *Locks::classlinker_classes_lock_);
+  Thread* const self = Thread::Current();
+  WriterMutexLock mu(self, *Locks::classlinker_classes_lock_);
   BufferedRootVisitor<kDefaultBufferedRootCount> buffered_visitor(
       visitor, RootInfo(kRootStickyClass));
   if ((flags & kVisitRootFlagAllRoots) != 0) {
@@ -1315,9 +1316,13 @@ void ClassLinker::VisitClassRoots(RootVisitor* visitor, VisitRootFlags flags) {
     // Need to make sure to not copy ArtMethods without doing read barriers since the roots are
     // marked concurrently and we don't hold the classlinker_classes_lock_ when we do the copy.
     boot_class_table_.VisitRoots(buffered_visitor);
-    for (GcRoot<mirror::ClassLoader>& root : class_loaders_) {
-      // May be null for boot ClassLoader.
-      root.VisitRoot(visitor, RootInfo(kRootVMInternal));
+    // TODO: Avoid marking these to enable class unloading.
+    JavaVMExt* const vm = Runtime::Current()->GetJavaVM();
+    for (jweak weak_root : class_loaders_) {
+      mirror::Object* class_loader =
+          down_cast<mirror::ClassLoader*>(vm->DecodeWeakGlobal(self, weak_root));
+      // Don't need to update anything since the class loaders will be updated by SweepSystemWeaks.
+      visitor->VisitRootIfNonNull(&class_loader, RootInfo(kRootVMInternal));
     }
   } else if ((flags & kVisitRootFlagNewRoots) != 0) {
     for (auto& root : new_class_roots_) {
@@ -1353,14 +1358,31 @@ void ClassLinker::VisitRoots(RootVisitor* visitor, VisitRootFlags flags) {
   }
 }
 
+class VisitClassLoaderClassesVisitor : public ClassLoaderVisitor {
+ public:
+  explicit VisitClassLoaderClassesVisitor(ClassVisitor* visitor)
+      : visitor_(visitor),
+        done_(false) {}
+
+  void Visit(mirror::ClassLoader* class_loader)
+      SHARED_REQUIRES(Locks::classlinker_classes_lock_, Locks::mutator_lock_) OVERRIDE {
+    ClassTable* const class_table = class_loader->GetClassTable();
+    if (!done_ && class_table != nullptr && !class_table->Visit(visitor_)) {
+      // If the visitor ClassTable returns false it means that we don't need to continue.
+      done_ = true;
+    }
+  }
+
+ private:
+  ClassVisitor* const visitor_;
+  // If done is true then we don't need to do any more visiting.
+  bool done_;
+};
+
 void ClassLinker::VisitClassesInternal(ClassVisitor* visitor) {
   if (boot_class_table_.Visit(visitor)) {
-    for (GcRoot<mirror::ClassLoader>& root : class_loaders_) {
-      ClassTable* const class_table = root.Read()->GetClassTable();
-      if (class_table != nullptr && !class_table->Visit(visitor)) {
-        return;
-      }
-    }
+    VisitClassLoaderClassesVisitor loader_visitor(visitor);
+    VisitClassLoaders(&loader_visitor);
   }
 }
 
@@ -1479,10 +1501,17 @@ ClassLinker::~ClassLinker() {
   mirror::LongArray::ResetArrayClass();
   mirror::ShortArray::ResetArrayClass();
   STLDeleteElements(&oat_files_);
-  for (GcRoot<mirror::ClassLoader>& root : class_loaders_) {
-    ClassTable* const class_table = root.Read()->GetClassTable();
-    delete class_table;
+  Thread* const self = Thread::Current();
+  JavaVMExt* const vm = Runtime::Current()->GetJavaVM();
+  for (jweak weak_root : class_loaders_) {
+    auto* const class_loader = down_cast<mirror::ClassLoader*>(
+        vm->DecodeWeakGlobal(self, weak_root));
+    if (class_loader != nullptr) {
+      delete class_loader->GetClassTable();
+    }
+    vm->DeleteWeakGlobalRef(self, weak_root);
   }
+  class_loaders_.clear();
 }
 
 mirror::PointerArray* ClassLinker::AllocPointerArray(Thread* self, size_t length) {
@@ -2611,8 +2640,7 @@ mirror::DexCache* ClassLinker::FindDexCacheLocked(Thread* self,
                                                   bool allow_failure) {
   // Search assuming unique-ness of dex file.
   JavaVMExt* const vm = self->GetJniEnv()->vm;
-  for (jobject weak_root : dex_caches_) {
-    DCHECK_EQ(GetIndirectRefKind(weak_root), kWeakGlobal);
+  for (jweak weak_root : dex_caches_) {
     mirror::DexCache* dex_cache = down_cast<mirror::DexCache*>(
         vm->DecodeWeakGlobal(self, weak_root));
     if (dex_cache != nullptr && dex_cache->GetDexFile() == &dex_file) {
@@ -2985,15 +3013,25 @@ void ClassLinker::MoveImageClassesToClassTable() {
   dex_cache_image_class_lookup_required_ = false;
 }
 
-void ClassLinker::MoveClassTableToPreZygote() {
-  WriterMutexLock mu(Thread::Current(), *Locks::classlinker_classes_lock_);
-  boot_class_table_.FreezeSnapshot();
-  for (GcRoot<mirror::ClassLoader>& root : class_loaders_) {
-    ClassTable* const class_table = root.Read()->GetClassTable();
+class MoveClassTableToPreZygoteVisitor : public ClassLoaderVisitor {
+ public:
+  explicit MoveClassTableToPreZygoteVisitor() {}
+
+  void Visit(mirror::ClassLoader* class_loader)
+      REQUIRES(Locks::classlinker_classes_lock_)
+      SHARED_REQUIRES(Locks::mutator_lock_) OVERRIDE {
+    ClassTable* const class_table = class_loader->GetClassTable();
     if (class_table != nullptr) {
       class_table->FreezeSnapshot();
     }
   }
+};
+
+void ClassLinker::MoveClassTableToPreZygote() {
+  WriterMutexLock mu(Thread::Current(), *Locks::classlinker_classes_lock_);
+  boot_class_table_.FreezeSnapshot();
+  MoveClassTableToPreZygoteVisitor visitor;
+  VisitClassLoadersAndRemoveClearedLoaders(&visitor);
 }
 
 mirror::Class* ClassLinker::LookupClassFromImage(const char* descriptor) {
@@ -3019,25 +3057,43 @@ mirror::Class* ClassLinker::LookupClassFromImage(const char* descriptor) {
   return nullptr;
 }
 
+// Look up classes by hash and descriptor and put all matching ones in the result array.
+class LookupClassesVisitor : public ClassLoaderVisitor {
+ public:
+  LookupClassesVisitor(const char* descriptor, size_t hash, std::vector<mirror::Class*>* result)
+     : descriptor_(descriptor),
+       hash_(hash),
+       result_(result) {}
+
+  void Visit(mirror::ClassLoader* class_loader)
+      SHARED_REQUIRES(Locks::classlinker_classes_lock_, Locks::mutator_lock_) OVERRIDE {
+    ClassTable* const class_table = class_loader->GetClassTable();
+    mirror::Class* klass = class_table->Lookup(descriptor_, hash_);
+    if (klass != nullptr) {
+      result_->push_back(klass);
+    }
+  }
+
+ private:
+  const char* const descriptor_;
+  const size_t hash_;
+  std::vector<mirror::Class*>* const result_;
+};
+
 void ClassLinker::LookupClasses(const char* descriptor, std::vector<mirror::Class*>& result) {
   result.clear();
   if (dex_cache_image_class_lookup_required_) {
     MoveImageClassesToClassTable();
   }
-  WriterMutexLock mu(Thread::Current(), *Locks::classlinker_classes_lock_);
+  Thread* const self = Thread::Current();
+  ReaderMutexLock mu(self, *Locks::classlinker_classes_lock_);
   const size_t hash = ComputeModifiedUtf8Hash(descriptor);
   mirror::Class* klass = boot_class_table_.Lookup(descriptor, hash);
   if (klass != nullptr) {
     result.push_back(klass);
   }
-  for (GcRoot<mirror::ClassLoader>& root : class_loaders_) {
-    // There can only be one class with the same descriptor per class loader.
-    ClassTable* const class_table = root.Read()->GetClassTable();
-    klass = class_table->Lookup(descriptor, hash);
-    if (klass != nullptr) {
-      result.push_back(klass);
-    }
-  }
+  LookupClassesVisitor visitor(descriptor, hash, &result);
+  VisitClassLoaders(&visitor);
 }
 
 void ClassLinker::VerifyClass(Thread* self, Handle<mirror::Class> klass) {
@@ -4109,7 +4165,8 @@ ClassTable* ClassLinker::InsertClassTableForClassLoader(mirror::ClassLoader* cla
   ClassTable* class_table = class_loader->GetClassTable();
   if (class_table == nullptr) {
     class_table = new ClassTable;
-    class_loaders_.push_back(class_loader);
+    Thread* const self = Thread::Current();
+    class_loaders_.push_back(self->GetJniEnv()->vm->AddWeakGlobalRef(self, class_loader));
     // Don't already have a class table, add it to the class loader.
     class_loader->SetClassTable(class_table);
   }
@@ -5875,26 +5932,33 @@ void ClassLinker::DumpForSigQuit(std::ostream& os) {
      << NumNonZygoteClasses() << "\n";
 }
 
-size_t ClassLinker::NumZygoteClasses() const {
-  size_t sum = boot_class_table_.NumZygoteClasses();
-  for (const GcRoot<mirror::ClassLoader>& root : class_loaders_) {
-    ClassTable* const class_table = root.Read()->GetClassTable();
+class CountClassesVisitor : public ClassLoaderVisitor {
+ public:
+  CountClassesVisitor() : num_zygote_classes(0), num_non_zygote_classes(0) {}
+
+  void Visit(mirror::ClassLoader* class_loader)
+      SHARED_REQUIRES(Locks::classlinker_classes_lock_, Locks::mutator_lock_) OVERRIDE {
+    ClassTable* const class_table = class_loader->GetClassTable();
     if (class_table != nullptr) {
-      sum += class_table->NumZygoteClasses();
+      num_zygote_classes += class_table->NumZygoteClasses();
+      num_non_zygote_classes += class_table->NumNonZygoteClasses();
     }
   }
-  return sum;
+
+  size_t num_zygote_classes;
+  size_t num_non_zygote_classes;
+};
+
+size_t ClassLinker::NumZygoteClasses() const {
+  CountClassesVisitor visitor;
+  VisitClassLoaders(&visitor);
+  return visitor.num_zygote_classes + boot_class_table_.NumZygoteClasses();
 }
 
 size_t ClassLinker::NumNonZygoteClasses() const {
-  size_t sum = boot_class_table_.NumNonZygoteClasses();
-  for (const GcRoot<mirror::ClassLoader>& root : class_loaders_) {
-    ClassTable* const class_table = root.Read()->GetClassTable();
-    if (class_table != nullptr) {
-      sum += class_table->NumNonZygoteClasses();
-    }
-  }
-  return sum;
+  CountClassesVisitor visitor;
+  VisitClassLoaders(&visitor);
+  return visitor.num_non_zygote_classes + boot_class_table_.NumNonZygoteClasses();
 }
 
 size_t ClassLinker::NumLoadedClasses() {
@@ -6107,4 +6171,35 @@ void ClassLinker::DropFindArrayClassCache() {
   find_array_class_cache_next_victim_ = 0;
 }
 
+void ClassLinker::VisitClassLoadersAndRemoveClearedLoaders(ClassLoaderVisitor* visitor) {
+  Thread* const self = Thread::Current();
+  Locks::classlinker_classes_lock_->AssertExclusiveHeld(self);
+  JavaVMExt* const vm = self->GetJniEnv()->vm;
+  for (auto it = class_loaders_.begin(); it != class_loaders_.end();) {
+    const jweak weak_root = *it;
+    mirror::ClassLoader* const class_loader = down_cast<mirror::ClassLoader*>(
+        vm->DecodeWeakGlobal(self, weak_root));
+    if (class_loader != nullptr) {
+      visitor->Visit(class_loader);
+      ++it;
+    } else {
+      // Remove the cleared weak reference from the array.
+      vm->DeleteWeakGlobalRef(self, weak_root);
+      it = class_loaders_.erase(it);
+    }
+  }
+}
+
+void ClassLinker::VisitClassLoaders(ClassLoaderVisitor* visitor) const {
+  Thread* const self = Thread::Current();
+  JavaVMExt* const vm = self->GetJniEnv()->vm;
+  for (jweak weak_root : class_loaders_) {
+    mirror::ClassLoader* const class_loader = down_cast<mirror::ClassLoader*>(
+        vm->DecodeWeakGlobal(self, weak_root));
+    if (class_loader != nullptr) {
+      visitor->Visit(class_loader);
+    }
+  }
+}
+
 }  // namespace art
diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index 17aa48a6f4..fee706625b 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -59,6 +59,13 @@ template<size_t kNumReferences> class PACKED(4) StackHandleScope;
 
 enum VisitRootFlags : uint8_t;
 
+class ClassLoaderVisitor {
+ public:
+  virtual ~ClassLoaderVisitor() {}
+  virtual void Visit(mirror::ClassLoader* class_loader)
+      SHARED_REQUIRES(Locks::classlinker_classes_lock_, Locks::mutator_lock_) = 0;
+};
+
 class ClassLinker {
  public:
   // Well known mirror::Class roots accessed via GetClassRoot.
@@ -540,8 +547,18 @@ class ClassLinker {
   void DropFindArrayClassCache() SHARED_REQUIRES(Locks::mutator_lock_);
 
  private:
+  // The RemoveClearedLoaders version removes cleared weak global class loaders and frees their
+  // class tables. This version can only be called with reader access to the
+  // classlinker_classes_lock_ since it modifies the class_loaders_ list.
+  void VisitClassLoadersAndRemoveClearedLoaders(ClassLoaderVisitor* visitor)
+      REQUIRES(Locks::classlinker_classes_lock_)
+      SHARED_REQUIRES(Locks::mutator_lock_);
+  void VisitClassLoaders(ClassLoaderVisitor* visitor) const
+      SHARED_REQUIRES(Locks::classlinker_classes_lock_, Locks::mutator_lock_);
+
+
   void VisitClassesInternal(ClassVisitor* visitor)
-      REQUIRES(Locks::classlinker_classes_lock_) SHARED_REQUIRES(Locks::mutator_lock_);
+      SHARED_REQUIRES(Locks::classlinker_classes_lock_, Locks::mutator_lock_);
 
   // Returns the number of zygote and image classes.
   size_t NumZygoteClasses() const
@@ -726,7 +743,7 @@ class ClassLinker {
   size_t GetDexCacheCount() SHARED_REQUIRES(Locks::mutator_lock_, dex_lock_) {
     return dex_caches_.size();
   }
-  const std::list<jobject>& GetDexCaches() SHARED_REQUIRES(Locks::mutator_lock_, dex_lock_) {
+  const std::list<jweak>& GetDexCaches() SHARED_REQUIRES(Locks::mutator_lock_, dex_lock_) {
     return dex_caches_;
   }
 
@@ -805,12 +822,12 @@ class ClassLinker {
   mutable ReaderWriterMutex dex_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER;
   // JNI weak globals to allow dex caches to get unloaded. We lazily delete weak globals when we
   // register new dex files.
-  std::list<jobject> dex_caches_ GUARDED_BY(dex_lock_);
+  std::list<jweak> dex_caches_ GUARDED_BY(dex_lock_);
   std::vector<const OatFile*> oat_files_ GUARDED_BY(dex_lock_);
 
-  // This contains the class laoders which have class tables. It is populated by
-  // InsertClassTableForClassLoader.
-  std::vector<GcRoot<mirror::ClassLoader>> class_loaders_
+  // This contains the class loaders which have class tables. It is populated by
+  // InsertClassTableForClassLoader. Weak roots to enable class unloading.
+  std::list<jweak> class_loaders_
       GUARDED_BY(Locks::classlinker_classes_lock_);
 
   // Boot class path table. Since the class loader for this is null.
diff --git a/runtime/class_table.h b/runtime/class_table.h
index 6b18d9009d..727392eb6f 100644
--- a/runtime/class_table.h
+++ b/runtime/class_table.h
@@ -58,10 +58,10 @@ class ClassTable {
       REQUIRES(Locks::classlinker_classes_lock_) SHARED_REQUIRES(Locks::mutator_lock_);
 
   // Returns the number of classes in previous snapshots.
-  size_t NumZygoteClasses() const REQUIRES(Locks::classlinker_classes_lock_);
+  size_t NumZygoteClasses() const SHARED_REQUIRES(Locks::classlinker_classes_lock_);
 
   // Returns all off the classes in the lastest snapshot.
-  size_t NumNonZygoteClasses() const REQUIRES(Locks::classlinker_classes_lock_);
+  size_t NumNonZygoteClasses() const SHARED_REQUIRES(Locks::classlinker_classes_lock_);
 
   // Update a class in the table with the new class. Returns the existing class which was replaced.
   mirror::Class* UpdateClass(const char* descriptor, mirror::Class* new_klass, size_t hash)
@@ -79,7 +79,7 @@ class ClassTable {
 
   // Return false if the callback told us to exit.
   bool Visit(ClassVisitor* visitor)
-      REQUIRES(Locks::classlinker_classes_lock_) SHARED_REQUIRES(Locks::mutator_lock_);
+      SHARED_REQUIRES(Locks::classlinker_classes_lock_, Locks::mutator_lock_);
 
   mirror::Class* Lookup(const char* descriptor, size_t hash)
       SHARED_REQUIRES(Locks::classlinker_classes_lock_, Locks::mutator_lock_);
diff --git a/runtime/gc_root.h b/runtime/gc_root.h
index 83471e6b96..477e67b3c2 100644
--- a/runtime/gc_root.h
+++ b/runtime/gc_root.h
@@ -90,16 +90,16 @@ class RootVisitor {
   virtual ~RootVisitor() { }
 
   // Single root version, not overridable.
-  ALWAYS_INLINE void VisitRoot(mirror::Object** roots, const RootInfo& info)
+  ALWAYS_INLINE void VisitRoot(mirror::Object** root, const RootInfo& info)
       SHARED_REQUIRES(Locks::mutator_lock_) {
-    VisitRoots(&roots, 1, info);
+    VisitRoots(&root, 1, info);
   }
 
   // Single root version, not overridable.
-  ALWAYS_INLINE void VisitRootIfNonNull(mirror::Object** roots, const RootInfo& info)
+  ALWAYS_INLINE void VisitRootIfNonNull(mirror::Object** root, const RootInfo& info)
       SHARED_REQUIRES(Locks::mutator_lock_) {
-    if (*roots != nullptr) {
-      VisitRoot(roots, info);
+    if (*root != nullptr) {
+      VisitRoot(root, info);
     }
   }
 
diff --git a/runtime/java_vm_ext.cc b/runtime/java_vm_ext.cc
index d6c798a863..92eef3983b 100644
--- a/runtime/java_vm_ext.cc
+++ b/runtime/java_vm_ext.cc
@@ -592,6 +592,7 @@ mirror::Object* JavaVMExt::DecodeWeakGlobal(Thread* self, IndirectRef ref) {
   // This only applies in the case where MayAccessWeakGlobals goes from false to true. In the other
   // case, it may be racy, this is benign since DecodeWeakGlobalLocked does the correct behavior
   // if MayAccessWeakGlobals is false.
+  DCHECK_EQ(GetIndirectRefKind(ref), kWeakGlobal);
   if (LIKELY(MayAccessWeakGlobalsUnlocked(self))) {
     return weak_globals_.SynchronizedGet(ref);
   }