summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMathieu Chartier <mathieuc@google.com>2017-06-01 11:26:50 -0700
committerMathieu Chartier <mathieuc@google.com>2017-06-02 09:38:14 -0700
commitfdd513d6c522841e82f1dc144d19a8d60269b9f7 (patch)
tree1ffbfd68c1271d158de32e1aa438906d3553256d
parent1656ca9e6996cb555b4463e5efd4bd7e3f4fb816 (diff)
downloadandroid_art-fdd513d6c522841e82f1dc144d19a8d60269b9f7.tar.gz
android_art-fdd513d6c522841e82f1dc144d19a8d60269b9f7.tar.bz2
android_art-fdd513d6c522841e82f1dc144d19a8d60269b9f7.zip
Move to release CAS for updating object fields
Relaxed cas is not sufficient to make sure threads that read the field will see the copied contents of objects. Bug: 37187694 Bug: 62240510 Test: test-art-host (cherry picked from commit a1f20c3f8d0dabb9723acccf3ba760acf3ebe62d) Change-Id: I8bff8a67c2c52eb131714b52e6d842c8c08dd70a
-rw-r--r--runtime/atomic.h7
-rw-r--r--runtime/class_table-inl.h4
-rw-r--r--runtime/gc/collector/concurrent_copying.cc7
-rw-r--r--runtime/mirror/object-inl.h30
-rw-r--r--runtime/mirror/object-readbarrier-inl.h30
-rw-r--r--runtime/mirror/object.h15
-rw-r--r--runtime/read_barrier-inl.h4
7 files changed, 91 insertions, 6 deletions
diff --git a/runtime/atomic.h b/runtime/atomic.h
index 45c3165b18..25dd1a3a5e 100644
--- a/runtime/atomic.h
+++ b/runtime/atomic.h
@@ -257,6 +257,13 @@ class PACKED(sizeof(T)) Atomic : public std::atomic<T> {
return this->compare_exchange_strong(expected_value, desired_value, std::memory_order_relaxed);
}
+ // Atomically replace the value with desired value if it matches the expected value. Prior writes
+ // to other memory locations become visible to the threads that do a consume or an acquire on the
+ // same location.
+ bool CompareExchangeStrongRelease(T expected_value, T desired_value) {
+ return this->compare_exchange_strong(expected_value, desired_value, std::memory_order_release);
+ }
+
// The same, except it may fail spuriously.
bool CompareExchangeWeakRelaxed(T expected_value, T desired_value) {
return this->compare_exchange_weak(expected_value, desired_value, std::memory_order_relaxed);
diff --git a/runtime/class_table-inl.h b/runtime/class_table-inl.h
index dfe8949134..35fce4063b 100644
--- a/runtime/class_table-inl.h
+++ b/runtime/class_table-inl.h
@@ -93,7 +93,7 @@ inline mirror::Class* ClassTable::TableSlot::Read() const {
if (kReadBarrierOption != kWithoutReadBarrier && before_ptr != after_ptr) {
// If another thread raced and updated the reference, do not store the read barrier updated
// one.
- data_.CompareExchangeStrongRelaxed(before, Encode(after_ptr, MaskHash(before)));
+ data_.CompareExchangeStrongRelease(before, Encode(after_ptr, MaskHash(before)));
}
return after_ptr.Ptr();
}
@@ -108,7 +108,7 @@ inline void ClassTable::TableSlot::VisitRoot(const Visitor& visitor) const {
if (before_ptr != after_ptr) {
// If another thread raced and updated the reference, do not store the read barrier updated
// one.
- data_.CompareExchangeStrongRelaxed(before, Encode(after_ptr, MaskHash(before)));
+ data_.CompareExchangeStrongRelease(before, Encode(after_ptr, MaskHash(before)));
}
}
diff --git a/runtime/gc/collector/concurrent_copying.cc b/runtime/gc/collector/concurrent_copying.cc
index 54882874c6..c0d648117c 100644
--- a/runtime/gc/collector/concurrent_copying.cc
+++ b/runtime/gc/collector/concurrent_copying.cc
@@ -2086,8 +2086,11 @@ inline void ConcurrentCopying::Process(mirror::Object* obj, MemberOffset offset)
// It was updated by the mutator.
break;
}
- } while (!obj->CasFieldWeakRelaxedObjectWithoutWriteBarrier<
- false, false, kVerifyNone>(offset, expected_ref, new_ref));
+ // Use release cas to make sure threads reading the reference see contents of copied objects.
+ } while (!obj->CasFieldWeakReleaseObjectWithoutWriteBarrier<false, false, kVerifyNone>(
+ offset,
+ expected_ref,
+ new_ref));
}
// Process some roots.
diff --git a/runtime/mirror/object-inl.h b/runtime/mirror/object-inl.h
index baed5f167c..d3fc95ff2d 100644
--- a/runtime/mirror/object-inl.h
+++ b/runtime/mirror/object-inl.h
@@ -899,6 +899,36 @@ inline bool Object::CasFieldWeakRelaxedObjectWithoutWriteBarrier(
return success;
}
+template<bool kTransactionActive, bool kCheckTransaction, VerifyObjectFlags kVerifyFlags>
+inline bool Object::CasFieldWeakReleaseObjectWithoutWriteBarrier(
+ MemberOffset field_offset,
+ ObjPtr<Object> old_value,
+ ObjPtr<Object> new_value) {
+ if (kCheckTransaction) {
+ DCHECK_EQ(kTransactionActive, Runtime::Current()->IsActiveTransaction());
+ }
+ if (kVerifyFlags & kVerifyThis) {
+ VerifyObject(this);
+ }
+ if (kVerifyFlags & kVerifyWrites) {
+ VerifyObject(new_value);
+ }
+ if (kVerifyFlags & kVerifyReads) {
+ VerifyObject(old_value);
+ }
+ if (kTransactionActive) {
+ Runtime::Current()->RecordWriteFieldReference(this, field_offset, old_value, true);
+ }
+ HeapReference<Object> old_ref(HeapReference<Object>::FromObjPtr(old_value));
+ HeapReference<Object> new_ref(HeapReference<Object>::FromObjPtr(new_value));
+ uint8_t* raw_addr = reinterpret_cast<uint8_t*>(this) + field_offset.Int32Value();
+ Atomic<uint32_t>* atomic_addr = reinterpret_cast<Atomic<uint32_t>*>(raw_addr);
+
+ bool success = atomic_addr->CompareExchangeWeakRelease(old_ref.reference_,
+ new_ref.reference_);
+ return success;
+}
+
template<bool kIsStatic,
VerifyObjectFlags kVerifyFlags,
ReadBarrierOption kReadBarrierOption,
diff --git a/runtime/mirror/object-readbarrier-inl.h b/runtime/mirror/object-readbarrier-inl.h
index 58e7c20667..69365af7fd 100644
--- a/runtime/mirror/object-readbarrier-inl.h
+++ b/runtime/mirror/object-readbarrier-inl.h
@@ -221,6 +221,36 @@ inline bool Object::CasFieldStrongRelaxedObjectWithoutWriteBarrier(
return success;
}
+template<bool kTransactionActive, bool kCheckTransaction, VerifyObjectFlags kVerifyFlags>
+inline bool Object::CasFieldStrongReleaseObjectWithoutWriteBarrier(
+ MemberOffset field_offset,
+ ObjPtr<Object> old_value,
+ ObjPtr<Object> new_value) {
+ if (kCheckTransaction) {
+ DCHECK_EQ(kTransactionActive, Runtime::Current()->IsActiveTransaction());
+ }
+ if (kVerifyFlags & kVerifyThis) {
+ VerifyObject(this);
+ }
+ if (kVerifyFlags & kVerifyWrites) {
+ VerifyObject(new_value);
+ }
+ if (kVerifyFlags & kVerifyReads) {
+ VerifyObject(old_value);
+ }
+ if (kTransactionActive) {
+ Runtime::Current()->RecordWriteFieldReference(this, field_offset, old_value, true);
+ }
+ HeapReference<Object> old_ref(HeapReference<Object>::FromObjPtr(old_value));
+ HeapReference<Object> new_ref(HeapReference<Object>::FromObjPtr(new_value));
+ uint8_t* raw_addr = reinterpret_cast<uint8_t*>(this) + field_offset.Int32Value();
+ Atomic<uint32_t>* atomic_addr = reinterpret_cast<Atomic<uint32_t>*>(raw_addr);
+
+ bool success = atomic_addr->CompareExchangeStrongRelease(old_ref.reference_,
+ new_ref.reference_);
+ return success;
+}
+
} // namespace mirror
} // namespace art
diff --git a/runtime/mirror/object.h b/runtime/mirror/object.h
index 35a1b733e1..9cf42522d1 100644
--- a/runtime/mirror/object.h
+++ b/runtime/mirror/object.h
@@ -350,10 +350,25 @@ class MANAGED LOCKABLE Object {
template<bool kTransactionActive,
bool kCheckTransaction = true,
VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
+ bool CasFieldWeakReleaseObjectWithoutWriteBarrier(MemberOffset field_offset,
+ ObjPtr<Object> old_value,
+ ObjPtr<Object> new_value)
+ REQUIRES_SHARED(Locks::mutator_lock_);
+
+ template<bool kTransactionActive,
+ bool kCheckTransaction = true,
+ VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
bool CasFieldStrongRelaxedObjectWithoutWriteBarrier(MemberOffset field_offset,
ObjPtr<Object> old_value,
ObjPtr<Object> new_value)
REQUIRES_SHARED(Locks::mutator_lock_);
+ template<bool kTransactionActive,
+ bool kCheckTransaction = true,
+ VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
+ bool CasFieldStrongReleaseObjectWithoutWriteBarrier(MemberOffset field_offset,
+ ObjPtr<Object> old_value,
+ ObjPtr<Object> new_value)
+ REQUIRES_SHARED(Locks::mutator_lock_);
template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
HeapReference<Object>* GetFieldObjectReferenceAddr(MemberOffset field_offset);
diff --git a/runtime/read_barrier-inl.h b/runtime/read_barrier-inl.h
index db774909dd..2d06e54f78 100644
--- a/runtime/read_barrier-inl.h
+++ b/runtime/read_barrier-inl.h
@@ -63,7 +63,7 @@ inline MirrorType* ReadBarrier::Barrier(
// If kAlwaysUpdateField is true, update the field atomically. This may fail if mutator
// updates before us, but it's OK.
if (kAlwaysUpdateField && ref != old_ref) {
- obj->CasFieldStrongRelaxedObjectWithoutWriteBarrier<false, false>(
+ obj->CasFieldStrongReleaseObjectWithoutWriteBarrier<false, false>(
offset, old_ref, ref);
}
}
@@ -81,7 +81,7 @@ inline MirrorType* ReadBarrier::Barrier(
ref = reinterpret_cast<MirrorType*>(Mark(old_ref));
// Update the field atomically. This may fail if mutator updates before us, but it's ok.
if (ref != old_ref) {
- obj->CasFieldStrongRelaxedObjectWithoutWriteBarrier<false, false>(
+ obj->CasFieldStrongReleaseObjectWithoutWriteBarrier<false, false>(
offset, old_ref, ref);
}
}