From 65f5f247a367af9d6b9ac63767b69ecf3ab079bc Mon Sep 17 00:00:00 2001 From: Hiroshi Yamauchi Date: Mon, 19 Dec 2016 11:44:47 -0800 Subject: Fix race condition btw DelayReferenceRefernent vs Reference.clear(). Rename IsMarkedHeapReference to IsNullOrMarkedHeapReference. Move the null check from the caller of IsMarkedHeapReference into IsNullOrMarkedHeapReference. Make sure that the referent is only loaded once between the null check and the IsMarked call. Use a CAS in ConcurrentCopying::IsNullOrMarkedHeapReference when called from DelayReferenceRefernent. Bug: 33389022 Test: test-art-host without and with CC. Change-Id: I20edab4dac2a4bb02dbb72af0f09de77b55ac08e --- runtime/gc/reference_queue.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'runtime/gc/reference_queue.cc') diff --git a/runtime/gc/reference_queue.cc b/runtime/gc/reference_queue.cc index a0eb197bd5..734caea371 100644 --- a/runtime/gc/reference_queue.cc +++ b/runtime/gc/reference_queue.cc @@ -129,8 +129,9 @@ void ReferenceQueue::ClearWhiteReferences(ReferenceQueue* cleared_references, while (!IsEmpty()) { ObjPtr ref = DequeuePendingReference(); mirror::HeapReference* referent_addr = ref->GetReferentReferenceAddr(); - if (referent_addr->AsMirrorPtr() != nullptr && - !collector->IsMarkedHeapReference(referent_addr)) { + // do_atomic_update is false because this happens during the reference processing phase where + // Reference.clear() would block. + if (!collector->IsNullOrMarkedHeapReference(referent_addr, /*do_atomic_update*/false)) { // Referent is white, clear it. if (Runtime::Current()->IsActiveTransaction()) { ref->ClearReferent(); @@ -147,8 +148,9 @@ void ReferenceQueue::EnqueueFinalizerReferences(ReferenceQueue* cleared_referenc while (!IsEmpty()) { ObjPtr ref = DequeuePendingReference()->AsFinalizerReference(); mirror::HeapReference* referent_addr = ref->GetReferentReferenceAddr(); - if (referent_addr->AsMirrorPtr() != nullptr && - !collector->IsMarkedHeapReference(referent_addr)) { + // do_atomic_update is false because this happens during the reference processing phase where + // Reference.clear() would block. + if (!collector->IsNullOrMarkedHeapReference(referent_addr, /*do_atomic_update*/false)) { ObjPtr forward_address = collector->MarkObject(referent_addr->AsMirrorPtr()); // Move the updated referent to the zombie field. if (Runtime::Current()->IsActiveTransaction()) { -- cgit v1.2.3-59-g8ed1b