Fix the DequeuePendingReference crash.

In DequeuePendingReference, acknowledge a black/white Reference object
in the queue if its referent was marked right after it's enqueued.

Bug: 12687968
Bug: 23896462
Change-Id: I33c802e04e1688a54a70ad3935628e3853c46e44

2 files changed, 34 insertions, 12 deletions

diff --git a/runtime/gc/collector/concurrent_copying.cc b/runtime/gc/collector/concurrent_copying.cc
index 57af95940d..4cbcdc5d42 100644
--- a/runtime/gc/collector/concurrent_copying.cc
+++ b/runtime/gc/collector/concurrent_copying.cc
@@ -1073,9 +1073,14 @@ void ConcurrentCopying::ProcessMarkStackRef(mirror::Object* to_ref) {
   if (to_ref->GetClass<kVerifyNone, kWithoutReadBarrier>()->IsTypeOfReferenceClass() &&
       to_ref->AsReference()->GetReferent<kWithoutReadBarrier>() != nullptr &&
       !IsInToSpace(to_ref->AsReference()->GetReferent<kWithoutReadBarrier>())) {
-    // Leave References gray so that GetReferent() will trigger RB.
+    // Leave this Reference gray in the queue so that GetReferent() will trigger a read barrier. We
+    // will change it to black or white later in ReferenceQueue::DequeuePendingReference().
     CHECK(to_ref->AsReference()->IsEnqueued()) << "Left unenqueued ref gray " << to_ref;
   } else {
+    // We may occasionally leave a Reference black or white in the queue if its referent happens to
+    // be concurrently marked after the Scan() call above has enqueued the Reference, in which case
+    // the above IsInToSpace() evaluates to true and we change the color from gray to black or white
+    // here in this else block.
 #ifdef USE_BAKER_OR_BROOKS_READ_BARRIER
     if (kUseBakerReadBarrier) {
       if (region_space_->IsInToSpace(to_ref)) {
diff --git a/runtime/gc/reference_queue.cc b/runtime/gc/reference_queue.cc
index f5054289e3..0c0ab6d212 100644
--- a/runtime/gc/reference_queue.cc
+++ b/runtime/gc/reference_queue.cc
@@ -89,19 +89,36 @@ mirror::Reference* ReferenceQueue::DequeuePendingReference() {
   Heap* heap = Runtime::Current()->GetHeap();
   if (kUseBakerOrBrooksReadBarrier && heap->CurrentCollectorType() == kCollectorTypeCC &&
       heap->ConcurrentCopyingCollector()->IsActive()) {
-    // Clear the gray ptr we left in ConcurrentCopying::ProcessMarkStack().
-    // We don't want to do this when the zygote compaction collector (SemiSpace) is running.
+    // Change the gray ptr we left in ConcurrentCopying::ProcessMarkStackRef() to black or white.
+    // We check IsActive() above because we don't want to do this when the zygote compaction
+    // collector (SemiSpace) is running.
     CHECK(ref != nullptr);
-    CHECK_EQ(ref->GetReadBarrierPointer(), ReadBarrier::GrayPtr())
-        << "ref=" << ref << " rb_ptr=" << ref->GetReadBarrierPointer();
-    if (heap->ConcurrentCopyingCollector()->RegionSpace()->IsInToSpace(ref)) {
-      // Moving objects.
-      ref->AtomicSetReadBarrierPointer(ReadBarrier::GrayPtr(), ReadBarrier::WhitePtr());
-      CHECK_EQ(ref->GetReadBarrierPointer(), ReadBarrier::WhitePtr());
+    collector::ConcurrentCopying* concurrent_copying = heap->ConcurrentCopyingCollector();
+    const bool is_moving = concurrent_copying->RegionSpace()->IsInToSpace(ref);
+    if (ref->GetReadBarrierPointer() == ReadBarrier::GrayPtr()) {
+      if (is_moving) {
+        ref->AtomicSetReadBarrierPointer(ReadBarrier::GrayPtr(), ReadBarrier::WhitePtr());
+        CHECK_EQ(ref->GetReadBarrierPointer(), ReadBarrier::WhitePtr());
+      } else {
+        ref->AtomicSetReadBarrierPointer(ReadBarrier::GrayPtr(), ReadBarrier::BlackPtr());
+        CHECK_EQ(ref->GetReadBarrierPointer(), ReadBarrier::BlackPtr());
+      }
     } else {
-      // Non-moving objects.
-      ref->AtomicSetReadBarrierPointer(ReadBarrier::GrayPtr(), ReadBarrier::BlackPtr());
-      CHECK_EQ(ref->GetReadBarrierPointer(), ReadBarrier::BlackPtr());
+      // In ConcurrentCopying::ProcessMarkStackRef() we may leave a black or white Reference in the
+      // queue and find it here, which is OK. Check that the color makes sense depending on whether
+      // the Reference is moving or not and that the referent has been marked.
+      if (is_moving) {
+        CHECK_EQ(ref->GetReadBarrierPointer(), ReadBarrier::WhitePtr())
+            << "ref=" << ref << " rb_ptr=" << ref->GetReadBarrierPointer();
+      } else {
+        CHECK_EQ(ref->GetReadBarrierPointer(), ReadBarrier::BlackPtr())
+            << "ref=" << ref << " rb_ptr=" << ref->GetReadBarrierPointer();
+      }
+      mirror::Object* referent = ref->GetReferent<kWithoutReadBarrier>();
+      CHECK(referent != nullptr) << "Reference should not have been enqueued if referent is null";
+      CHECK(concurrent_copying->IsInToSpace(referent))
+          << "ref=" << ref << " rb_ptr=" << ref->GetReadBarrierPointer()
+          << " referent=" << referent;
     }
   }
   return ref;