commit 46a23638436afdf17330870ef150f5b8eb66410c
author Mathieu Chartier <mathieuc@google.com> Tue Aug 07 18:44:40 2012 -0700
committer Mathieu Chartier <mathieuc@google.com> Wed Aug 08 09:47:05 2012 -0700
tree d9dec192977e0bf75a1bc7ada5b3a90ec9333345
parent 77ae36b35d47393335bf5399cab9c91ccf08e88f

Fix SweepSystemWeaks not checking the correct bitmaps due to bitmap swapping.

This error was causing GCs to occasionally free weak objects which were still in use (Monitors, etc..).

Change-Id: I2f4cfee1e76f2bb01d745ec8316774205f69c42f

src/mark_sweep.cc

diff --git a/src/mark_sweep.cc b/src/mark_sweep.cc
index df394db..7adc344 100644
--- a/src/mark_sweep.cc
+++ b/src/mark_sweep.cc
@@ -294,23 +294,26 @@
   Runtime::Current()->VisitRoots(ReMarkObjectVisitor, this);
 }
 
-void MarkSweep::SweepJniWeakGlobals() {
+void MarkSweep::SweepJniWeakGlobals(HeapBitmap* bitmap) {
   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 (!heap_->GetMarkBitmap()->Test(*entry)) {
+    if (!bitmap->Test(*entry)) {
       *entry = kClearedJniWeakGlobal;
     }
   }
 }
 
-void MarkSweep::SweepSystemWeaks() {
-  Runtime::Current()->GetInternTable()->SweepInternTableWeaks(IsMarked, this);
-  Runtime::Current()->GetMonitorList()->SweepMonitorList(IsMarked, this);
-  SweepJniWeakGlobals();
+void MarkSweep::SweepSystemWeaks(bool swap_bitmaps) {
+  Runtime* runtime = Runtime::Current();
+  runtime->GetInternTable()->SweepInternTableWeaks(swap_bitmaps ? IsLiveCallback : IsMarkedCallback,
+                                                   this);
+  runtime->GetMonitorList()->SweepMonitorList(swap_bitmaps ? IsLiveCallback : IsMarkedCallback,
+                                              this);
+  SweepJniWeakGlobals(swap_bitmaps ? GetHeap()->GetLiveBitmap() : GetHeap()->GetMarkBitmap());
 }
 
 struct SweepCallbackContext {
@@ -361,11 +364,12 @@
 }
 
 void MarkSweep::Sweep(bool partial) {
-  SweepSystemWeaks();
+  // If we don't swap bitmaps then we can not do this concurrently.
+  SweepSystemWeaks(true);
 
   DCHECK(mark_stack_->IsEmpty());
 
-  const std::vector<Space*>& spaces = heap_->GetSpaces();
+  const Spaces& spaces = heap_->GetSpaces();
   SweepCallbackContext scc;
   scc.heap = heap_;
   for (size_t i = 0; i < spaces.size(); ++i) {

src/mark_sweep.h

diff --git a/src/mark_sweep.h b/src/mark_sweep.h
index 108da87..189462f 100644
--- a/src/mark_sweep.h
+++ b/src/mark_sweep.h
@@ -99,10 +99,14 @@
     return heap_->GetMarkBitmap()->Test(object);
   }
 
-  static bool IsMarked(const Object* object, void* arg) {
+  static bool IsMarkedCallback(const Object* object, void* arg) {
     return reinterpret_cast<MarkSweep*>(arg)->IsMarked(object);
   }
 
+  static bool IsLiveCallback(const Object* object, void* arg) {
+    return reinterpret_cast<MarkSweep*>(arg)->GetHeap()->GetLiveBitmap()->Test(object);
+  }
+
   static void MarkObjectVisitor(const Object* root, void* arg);
 
   static void ReMarkObjectVisitor(const Object* root, void* arg);
@@ -250,8 +254,8 @@
                          Object** finalizer_references,
                          Object** phantom_references);
 
-  void SweepSystemWeaks();
-  void SweepJniWeakGlobals();
+  void SweepSystemWeaks(bool swap_bitmaps);
+  void SweepJniWeakGlobals(HeapBitmap* bitmap);
 
   // Current space, we check this space first to avoid searching for the appropriate space for an object.
   SpaceBitmap* current_mark_bitmap_;