commit 10d2508b105427ef1bcaf0c222873bae7acc66d3
author Mathieu Chartier <mathieuc@google.com> Wed Oct 28 18:36:09 2015 -0700
committer Mathieu Chartier <mathieuc@google.com> Wed Oct 28 19:24:54 2015 -0700
tree 8aa018dd07a1d84daf9f44ea7bb1c02442c44097
parent 5f2e628b280dfc5f8385e489d44cc0960fcea484

Change Checkpoint API to return total number of checkpoints

Fixes a race condition with SetStateUnsafe that caused some
warnings in the Barrier::~Barrier.

The race was:
RunCheckpoint sees suspended thread, runs the checkpoint. Inside the
checkpoint, the thread state had changed to runnable by
SetStateUnsafe. This occasionally caused more Barrier::Pass than
expected.

The fix is to return the total number of checkpoints instead of just
the runnable ones.

Bug: 24191051
Change-Id: If15a933ed4c8efa66a5f27cd5feaa2e5957ae804

runtime/gc/collector/concurrent_copying.cc

diff --git a/runtime/gc/collector/concurrent_copying.cc b/runtime/gc/collector/concurrent_copying.cc
index e433b8d..d7e8f81 100644
--- a/runtime/gc/collector/concurrent_copying.cc
+++ b/runtime/gc/collector/concurrent_copying.cc
@@ -339,9 +339,7 @@
         << thread->GetState() << " thread " << thread << " self " << self;
     // If thread is a running mutator, then act on behalf of the garbage collector.
     // See the code in ThreadList::RunCheckpoint.
-    if (thread->GetState() == kRunnable) {
-      concurrent_copying_->GetBarrier().Pass(self);
-    }
+    concurrent_copying_->GetBarrier().Pass(self);
   }
 
  private:
@@ -514,9 +512,7 @@
     thread->SetIsGcMarking(false);
     // If thread is a running mutator, then act on behalf of the garbage collector.
     // See the code in ThreadList::RunCheckpoint.
-    if (thread->GetState() == kRunnable) {
-      concurrent_copying_->GetBarrier().Pass(self);
-    }
+    concurrent_copying_->GetBarrier().Pass(self);
   }
 
  private:
@@ -937,9 +933,7 @@
     }
     // If thread is a running mutator, then act on behalf of the garbage collector.
     // See the code in ThreadList::RunCheckpoint.
-    if (thread->GetState() == kRunnable) {
-      concurrent_copying_->GetBarrier().Pass(self);
-    }
+    concurrent_copying_->GetBarrier().Pass(self);
   }
 
  private:

runtime/gc/collector/mark_sweep.cc

diff --git a/runtime/gc/collector/mark_sweep.cc b/runtime/gc/collector/mark_sweep.cc
index 77a288b..db516a0 100644
--- a/runtime/gc/collector/mark_sweep.cc
+++ b/runtime/gc/collector/mark_sweep.cc
@@ -1146,9 +1146,7 @@
     }
     // If thread is a running mutator, then act on behalf of the garbage collector.
     // See the code in ThreadList::RunCheckpoint.
-    if (thread->GetState() == kRunnable) {
-      mark_sweep_->GetBarrier().Pass(self);
-    }
+    mark_sweep_->GetBarrier().Pass(self);
   }
 
  private:

runtime/gc/heap.cc

diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc
index 1d38525..ab93142 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -1291,9 +1291,7 @@
     ATRACE_END();
     // If thread is a running mutator, then act on behalf of the trim thread.
     // See the code in ThreadList::RunCheckpoint.
-    if (thread->GetState() == kRunnable) {
-      barrier_->Pass(Thread::Current());
-    }
+    barrier_->Pass(Thread::Current());
   }
 
  private: