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
diff --git a/runtime/gc/collector/concurrent_copying.cc b/runtime/gc/collector/concurrent_copying.cc
index 57af959..4cbcdc5 100644
--- a/runtime/gc/collector/concurrent_copying.cc
+++ b/runtime/gc/collector/concurrent_copying.cc
@@ -1073,9 +1073,14 @@
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 f505428..0c0ab6d 100644
--- a/runtime/gc/reference_queue.cc
+++ b/runtime/gc/reference_queue.cc
@@ -89,19 +89,36 @@
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;
