Mark more roots.

This is most of the stuff. (Everything that currently exists, though there's
more to come.)

Change-Id: I235a21b006820a027c494374a5b52ffefed89c32

18 files changed, 159 insertions, 59 deletions

diff --git a/src/class_linker.cc b/src/class_linker.cc
index da87a9744b..36bf6a7300 100644
--- a/src/class_linker.cc
+++ b/src/class_linker.cc
@@ -426,25 +426,22 @@ void ClassLinker::InitCallback(Object* obj, void *arg) {
 // Keep in sync with InitCallback. Anything we visit, we need to
 // reinit references to when reinitializing a ClassLinker from a
 // mapped image.
-void ClassLinker::VisitRoots(Heap::RootVistor* root_visitor, void* arg) const {
-
-  root_visitor(class_roots_, arg);
+void ClassLinker::VisitRoots(Heap::RootVisitor* visitor, void* arg) const {
+  visitor(class_roots_, arg);
 
   for (size_t i = 0; i < dex_caches_.size(); i++) {
-      root_visitor(dex_caches_[i], arg);
+    visitor(dex_caches_[i], arg);
   }
 
   {
     MutexLock mu(classes_lock_);
     typedef Table::const_iterator It; // TODO: C++0x auto
     for (It it = classes_.begin(), end = classes_.end(); it != end; ++it) {
-        root_visitor(it->second, arg);
+      visitor(it->second, arg);
     }
   }
 
-  intern_table_->VisitRoots(root_visitor, arg);
-
-  root_visitor(array_interfaces_, arg);
+  visitor(array_interfaces_, arg);
 }
 
 ClassLinker::~ClassLinker() {
diff --git a/src/class_linker.h b/src/class_linker.h
index 4702a574ba..3f95b8d7bd 100644
--- a/src/class_linker.h
+++ b/src/class_linker.h
@@ -122,7 +122,7 @@ class ClassLinker {
     return boot_class_path_;
   }
 
-  void VisitRoots(Heap::RootVistor* root_visitor, void* arg) const;
+  void VisitRoots(Heap::RootVisitor* visitor, void* arg) const;
 
   const DexFile& FindDexFile(const DexCache* dex_cache) const;
   DexCache* FindDexCache(const DexFile& dex_file) const;
diff --git a/src/heap.h b/src/heap.h
index c3ea080c77..6420a2ad8f 100644
--- a/src/heap.h
+++ b/src/heap.h
@@ -7,11 +7,11 @@
 
 #include "globals.h"
 #include "object_bitmap.h"
-#include "thread.h"
 
 namespace art {
 
 class Class;
+class Mutex;
 class Object;
 class Space;
 class HeapBitmap;
@@ -22,7 +22,7 @@ class Heap {
 
   static const size_t kMaximumSize = 64 * MB;
 
-  typedef void (RootVistor)(const Object* root, void* arg);
+  typedef void (RootVisitor)(const Object* root, void* arg);
 
   // Create a heap with the requested sizes. optional boot image may
   // be NULL, otherwise it is an image filename created by ImageWriter.
@@ -175,19 +175,6 @@ class Heap {
   DISALLOW_IMPLICIT_CONSTRUCTORS(Heap);
 };
 
-class HeapLock {
- public:
-  HeapLock(Heap* heap) : lock_(heap->GetLock()) {
-    lock_->Lock();
-  }
-  ~HeapLock() {
-    lock_->Unlock();
-  }
- private:
-  Mutex* lock_;
-  DISALLOW_COPY_AND_ASSIGN(HeapLock);
-};
-
 }  // namespace art
 
 #endif  // ART_SRC_HEAP_H_
diff --git a/src/indirect_reference_table.cc b/src/indirect_reference_table.cc
index 32d942f20e..e3632a660b 100644
--- a/src/indirect_reference_table.cc
+++ b/src/indirect_reference_table.cc
@@ -295,6 +295,13 @@ bool IndirectReferenceTable::Remove(uint32_t cookie, IndirectRef iref) {
   return true;
 }
 
+void IndirectReferenceTable::VisitRoots(Heap::RootVisitor* visitor, void* arg) {
+  typedef IndirectReferenceTable::iterator It; // TODO: C++0x auto
+  for (It it = begin(), end = this->end(); it != end; ++it) {
+    visitor(**it, arg);
+  }
+}
+
 std::ostream& operator<<(std::ostream& os, IndirectRefKind rhs) {
   switch (rhs) {
   case kSirtOrInvalid:
diff --git a/src/indirect_reference_table.h b/src/indirect_reference_table.h
index da5f955310..a53983f948 100644
--- a/src/indirect_reference_table.h
+++ b/src/indirect_reference_table.h
@@ -17,6 +17,7 @@
 #ifndef ART_SRC_INDIRECT_REFERENCE_TABLE_H_
 #define ART_SRC_INDIRECT_REFERENCE_TABLE_H_
 
+#include "heap.h"
 #include "logging.h"
 
 #include <iosfwd>
@@ -307,6 +308,8 @@ class IndirectReferenceTable {
     return iterator(table_, Capacity(), Capacity());
   }
 
+  void VisitRoots(Heap::RootVisitor* visitor, void* arg);
+
  private:
   /*
    * Extract the table index from an indirect reference.
diff --git a/src/intern_table.cc b/src/intern_table.cc
index 7d5be299cc..8a4e87e106 100644
--- a/src/intern_table.cc
+++ b/src/intern_table.cc
@@ -20,11 +20,11 @@ size_t InternTable::Size() const {
   return strong_interns_.size() + weak_interns_.size();
 }
 
-void InternTable::VisitRoots(Heap::RootVistor* root_visitor, void* arg) const {
+void InternTable::VisitRoots(Heap::RootVisitor* visitor, void* arg) const {
   MutexLock mu(intern_table_lock_);
   typedef Table::const_iterator It; // TODO: C++0x auto
   for (It it = strong_interns_.begin(), end = strong_interns_.end(); it != end; ++it) {
-    root_visitor(it->second, arg);
+    visitor(it->second, arg);
   }
   // Note: we deliberately don't visit the weak_interns_ table.
 }
@@ -116,7 +116,7 @@ bool InternTable::ContainsWeak(const String* s) {
   return found == s;
 }
 
-void InternTable::RemoveWeakIf(bool (*predicate)(const String*)) {
+void InternTable::RemoveWeakIf(const Predicate& predicate) {
   MutexLock mu(intern_table_lock_);
   typedef Table::const_iterator It; // TODO: C++0x auto
   for (It it = weak_interns_.begin(), end = weak_interns_.end(); it != end;) {
diff --git a/src/intern_table.h b/src/intern_table.h
index d7e9839a6c..c9de228315 100644
--- a/src/intern_table.h
+++ b/src/intern_table.h
@@ -35,14 +35,20 @@ class InternTable {
   // Used when reinitializing InternTable from an image.
   void RegisterStrong(const String* s);
 
-  // Removes all weak interns for which 'predicate' is true.
-  void RemoveWeakIf(bool (*predicate)(const String*));
+  // Removes all weak interns for which the predicate functor 'p' returns true.
+  // (We use an explicit Predicate type rather than a template to keep implementation
+  // out of the header file.)
+  struct Predicate {
+    virtual ~Predicate() {}
+    virtual bool operator()(const String*) const = 0;
+  };
+  void RemoveWeakIf(const Predicate& p);
 
   bool ContainsWeak(const String* s);
 
   size_t Size() const;
 
-  void VisitRoots(Heap::RootVistor* root_visitor, void* arg) const;
+  void VisitRoots(Heap::RootVisitor* visitor, void* arg) const;
 
  private:
   typedef std::tr1::unordered_multimap<int32_t, const String*> Table;
diff --git a/src/intern_table_test.cc b/src/intern_table_test.cc
index d230a01736..0362b0522c 100644
--- a/src/intern_table_test.cc
+++ b/src/intern_table_test.cc
@@ -36,20 +36,33 @@ TEST_F(InternTableTest, Size) {
   EXPECT_EQ(2U, t.Size());
 }
 
-std::vector<const String*> gExpectedWeakStrings;
-bool TestPredicate(const String* s) {
-  bool erased = false;
-  typedef std::vector<const String*>::iterator It; // TODO: C++0x auto
-  for (It it = gExpectedWeakStrings.begin(), end = gExpectedWeakStrings.end(); it != end; ++it) {
-    if (*it == s) {
-      gExpectedWeakStrings.erase(it);
-      erased = true;
-      break;
+class TestPredicate : public InternTable::Predicate {
+ public:
+  bool operator()(const String* s) const {
+    bool erased = false;
+    typedef std::vector<const String*>::iterator It; // TODO: C++0x auto
+    for (It it = expected_.begin(), end = expected_.end(); it != end; ++it) {
+      if (*it == s) {
+        expected_.erase(it);
+        erased = true;
+        break;
+      }
     }
+    EXPECT_TRUE(erased);
+    return true;
   }
-  EXPECT_TRUE(erased);
-  return true;
-}
+
+  void Expect(const String* s) {
+    expected_.push_back(s);
+  }
+
+  ~TestPredicate() {
+    EXPECT_EQ(0U, expected_.size());
+  }
+
+ private:
+  mutable std::vector<const String*> expected_;
+};
 
 TEST_F(InternTableTest, RemoveWeakIf) {
   InternTable t;
@@ -61,11 +74,10 @@ TEST_F(InternTableTest, RemoveWeakIf) {
   EXPECT_EQ(4U, t.Size());
 
   // We should traverse only the weaks...
-  gExpectedWeakStrings.clear();
-  gExpectedWeakStrings.push_back(s0);
-  gExpectedWeakStrings.push_back(s1);
-  t.RemoveWeakIf(TestPredicate);
-  EXPECT_EQ(0U, gExpectedWeakStrings.size());
+  TestPredicate p;
+  p.Expect(s0);
+  p.Expect(s1);
+  t.RemoveWeakIf(p);
 
   EXPECT_EQ(2U, t.Size());
 
diff --git a/src/jni_internal.cc b/src/jni_internal.cc
index 685ac2edc9..c9e4839e49 100644
--- a/src/jni_internal.cc
+++ b/src/jni_internal.cc
@@ -2862,6 +2862,18 @@ void* JavaVMExt::FindCodeForNativeMethod(Method* m) {
   return libraries->FindNativeMethod(m);
 }
 
+void JavaVMExt::VisitRoots(Heap::RootVisitor* visitor, void* arg) {
+  {
+    MutexLock mu(globals_lock);
+    globals.VisitRoots(visitor, arg);
+  }
+  {
+    MutexLock mu(pins_lock);
+    pin_table.VisitRoots(visitor, arg);
+  }
+  // The weak_globals table is visited by the GC itself (because it mutates the table).
+}
+
 }  // namespace art
 
 std::ostream& operator<<(std::ostream& os, const jobjectRefType& rhs) {
diff --git a/src/jni_internal.h b/src/jni_internal.h
index 74c107ceb5..11f7750d13 100644
--- a/src/jni_internal.h
+++ b/src/jni_internal.h
@@ -5,6 +5,7 @@
 
 #include "jni.h"
 
+#include "heap.h"
 #include "indirect_reference_table.h"
 #include "macros.h"
 #include "reference_table.h"
@@ -43,6 +44,8 @@ struct JavaVMExt : public JavaVM {
    */
   void* FindCodeForNativeMethod(Method* m);
 
+  void VisitRoots(Heap::RootVisitor*, void*);
+
   Runtime* runtime;
 
   // Used for testing. By default, we'll LOG(FATAL) the reason.
diff --git a/src/mark_sweep.cc b/src/mark_sweep.cc
index 0b61b9ab65..af7f1a5015 100644
--- a/src/mark_sweep.cc
+++ b/src/mark_sweep.cc
@@ -5,12 +5,14 @@
 #include <climits>
 #include <vector>
 
+#include "class_loader.h"
 #include "heap.h"
+#include "indirect_reference_table.h"
+#include "intern_table.h"
 #include "logging.h"
 #include "macros.h"
 #include "mark_stack.h"
 #include "object.h"
-#include "class_loader.h"
 #include "runtime.h"
 #include "space.h"
 #include "thread.h"
@@ -107,11 +109,38 @@ void MarkSweep::ReMarkRoots() {
   UNIMPLEMENTED(FATAL);
 }
 
-void MarkSweep::SweepSystemWeaks() {
-  //Runtime::Current()->GetInternTable().RemoveWeakIf(isUnmarkedObject);
+void MarkSweep::SweepJniWeakGlobals() {
+  JavaVMExt* vm = Runtime::Current()->GetJavaVM();
+  MutexLock mu(vm->weak_globals_lock);
+  IndirectReferenceTable* table = &vm->weak_globals;
+  typedef IndirectReferenceTable::iterator It; // TODO: C++0x auto
+  for (It it = table->begin(), end = table->end(); it != end; ++it) {
+    const Object** entry = *it;
+    if (!IsMarked(*entry)) {
+      *entry = kClearedJniWeakGlobal;
+    }
+  }
+}
+
+struct InternTableEntryIsUnmarked : public InternTable::Predicate {
+  InternTableEntryIsUnmarked(MarkSweep* ms) : ms_(ms) { }
+
+  bool operator()(const String* s) const {
+    return !ms_->IsMarked(s);
+  }
+
+  MarkSweep* ms_;
+};
+
+void MarkSweep::SweepMonitorList() {
   UNIMPLEMENTED(FATAL);
   //dvmSweepMonitorList(&gDvm.monitorList, isUnmarkedObject);
-  //sweepWeakJniGlobals();
+}
+
+void MarkSweep::SweepSystemWeaks() {
+  Runtime::Current()->GetInternTable()->RemoveWeakIf(InternTableEntryIsUnmarked(this));
+  SweepMonitorList();
+  SweepJniWeakGlobals();
 }
 
 void MarkSweep::SweepCallback(size_t num_ptrs, void **ptrs, void *arg) {
diff --git a/src/mark_sweep.h b/src/mark_sweep.h
index 047e357957..f20cc416bf 100644
--- a/src/mark_sweep.h
+++ b/src/mark_sweep.h
@@ -127,6 +127,8 @@ class MarkSweep {
                          Object** phantom_references);
 
   void SweepSystemWeaks();
+  void SweepMonitorList();
+  void SweepJniWeakGlobals();
 
   MarkStack* mark_stack_;
 
@@ -148,6 +150,8 @@ class MarkSweep {
 
   Object* cleared_reference_list_;
 
+  friend class InternTableEntryIsUnmarked;
+
   DISALLOW_COPY_AND_ASSIGN(MarkSweep);
 };
 
diff --git a/src/reference_table.cc b/src/reference_table.cc
index a006b65eee..df3daf2499 100644
--- a/src/reference_table.cc
+++ b/src/reference_table.cc
@@ -135,7 +135,7 @@ void ReferenceTable::Dump() const {
   Dump(entries_);
 }
 
-void ReferenceTable::Dump(const std::vector<const Object*>& entries) {
+void ReferenceTable::Dump(const Table& entries) {
   if (entries.empty()) {
     LOG(WARNING) << "  (empty)";
     return;
@@ -195,7 +195,7 @@ void ReferenceTable::Dump(const std::vector<const Object*>& entries) {
   }
 
   // Make a copy of the table and sort it.
-  std::vector<const Object*> sorted_entries(entries.begin(), entries.end());
+  Table sorted_entries(entries.begin(), entries.end());
   std::sort(sorted_entries.begin(), sorted_entries.end(), ObjectComparator());
 
   // Remove any uninteresting stuff from the list. The sort moved them all to the end.
@@ -233,4 +233,11 @@ void ReferenceTable::Dump(const std::vector<const Object*>& entries) {
   LogSummaryLine(sorted_entries.back(), GetElementCount(sorted_entries.back()), identical, equiv);
 }
 
+void ReferenceTable::VisitRoots(Heap::RootVisitor* visitor, void* arg) {
+  typedef Table::const_iterator It; // TODO: C++0x auto
+  for (It it = entries_.begin(), end = entries_.end(); it != end; ++it) {
+    visitor(*it, arg);
+  }
+}
+
 }  // namespace art
diff --git a/src/reference_table.h b/src/reference_table.h
index 13317ea69c..9142b7c8db 100644
--- a/src/reference_table.h
+++ b/src/reference_table.h
@@ -22,6 +22,8 @@
 #include <string>
 #include <vector>
 
+#include "heap.h"
+
 namespace art {
 
 class Object;
@@ -43,12 +45,15 @@ class ReferenceTable {
 
   void Dump() const;
 
+  void VisitRoots(Heap::RootVisitor* visitor, void* arg);
+
  private:
-  static void Dump(const std::vector<const Object*>& entries);
+  typedef std::vector<const Object*> Table;
+  static void Dump(const Table& entries);
   friend class IndirectReferenceTable; // For Dump.
 
   std::string name_;
-  std::vector<const Object*> entries_;
+  Table entries_;
   size_t max_size_;
 };
 
diff --git a/src/runtime.cc b/src/runtime.cc
index 985cdf6486..0e5da23240 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -433,9 +433,16 @@ bool Runtime::DetachCurrentThread() {
   return true;
 }
 
-void Runtime::VisitRoots(Heap::RootVistor* root_visitor, void* arg) const {
-  GetClassLinker()->VisitRoots(root_visitor, arg);
-  UNIMPLEMENTED(WARNING) << "mark other roots including per thread state and thread stacks";
+void Runtime::VisitRoots(Heap::RootVisitor* visitor, void* arg) const {
+  class_linker_->VisitRoots(visitor, arg);
+  intern_table_->VisitRoots(visitor, arg);
+  java_vm_->VisitRoots(visitor, arg);
+  thread_list_->VisitRoots(visitor, arg);
+
+  //(*visitor)(&gDvm.outOfMemoryObj, 0, ROOT_VM_INTERNAL, arg);
+  //(*visitor)(&gDvm.internalErrorObj, 0, ROOT_VM_INTERNAL, arg);
+  //(*visitor)(&gDvm.noClassDefFoundErrorObj, 0, ROOT_VM_INTERNAL, arg);
+  UNIMPLEMENTED(WARNING) << "some roots not marked";
 }
 
 }  // namespace art
diff --git a/src/runtime.h b/src/runtime.h
index 922eeab838..6a5d755835 100644
--- a/src/runtime.h
+++ b/src/runtime.h
@@ -101,7 +101,7 @@ class Runtime {
     return java_vm_;
   }
 
-  void VisitRoots(Heap::RootVistor* root_visitor, void* arg) const;
+  void VisitRoots(Heap::RootVisitor* visitor, void* arg) const;
 
  private:
   static void PlatformAbort(const char*, int);
diff --git a/src/thread.cc b/src/thread.cc
index 89ec844c69..5a612dd981 100644
--- a/src/thread.cc
+++ b/src/thread.cc
@@ -522,6 +522,15 @@ void* Thread::FindExceptionHandlerInMethod(const Method* method,
   }
 }
 
+void Thread::VisitRoots(Heap::RootVisitor* visitor, void* arg) const {
+  //(*visitor)(&thread->threadObj, threadId, ROOT_THREAD_OBJECT, arg);
+  //(*visitor)(&thread->exception, threadId, ROOT_NATIVE_STACK, arg);
+  jni_env_->locals.VisitRoots(visitor, arg);
+  jni_env_->monitors.VisitRoots(visitor, arg);
+  // visitThreadStack(visitor, thread, arg);
+  UNIMPLEMENTED(WARNING) << "some per-Thread roots not visited";
+}
+
 static const char* kStateNames[] = {
   "New",
   "Runnable",
@@ -588,4 +597,12 @@ void ThreadList::Unregister(Thread* thread) {
   list_.remove(thread);
 }
 
+void ThreadList::VisitRoots(Heap::RootVisitor* visitor, void* arg) const {
+  MutexLock mu(lock_);
+  typedef std::list<Thread*>::const_iterator It; // TODO: C++0x auto
+  for (It it = list_.begin(), end = list_.end(); it != end; ++it) {
+    (*it)->VisitRoots(visitor, arg);
+  }
+}
+
 }  // namespace
diff --git a/src/thread.h b/src/thread.h
index ab0660e40e..ae07fa607d 100644
--- a/src/thread.h
+++ b/src/thread.h
@@ -406,6 +406,8 @@ class Thread {
   // Allocate stack trace
   ObjectArray<StackTraceElement>* AllocStackTrace();
 
+  void VisitRoots(Heap::RootVisitor* visitor, void* arg) const;
+
  private:
   Thread()
       : id_(1234),
@@ -509,6 +511,8 @@ class ThreadList {
     lock_->Unlock();
   };
 
+  void VisitRoots(Heap::RootVisitor* visitor, void* arg) const;
+
  private:
   ThreadList();