Fix infinite loop in GenerateIdentityHashCode
Root Cause:
If no one changes the seed, it will become infinite loop due to below condition
(expected_value & LockWord::kHashMask) == 0
Solution:
Changes the seed before entering the next loop
Added test.
Bug: 19046417
(cherry picked from commit 7380c3175b443cdc6f12a2a3a91df188eaaa5a61)
Change-Id: I7d1c377dd1bda780681514b24d61ebc776bc80ab
diff --git a/runtime/mirror/object.cc b/runtime/mirror/object.cc
index 07bdf43..d2cc367 100644
--- a/runtime/mirror/object.cc
+++ b/runtime/mirror/object.cc
@@ -39,6 +39,8 @@
namespace art {
namespace mirror {
+Atomic<uint32_t> Object::hash_code_seed(987654321U + std::time(nullptr));
+
class CopyReferenceFieldsWithReadBarrierVisitor {
public:
explicit CopyReferenceFieldsWithReadBarrierVisitor(Object* dest_obj)
@@ -135,17 +137,20 @@
return copy;
}
-int32_t Object::GenerateIdentityHashCode() {
- static AtomicInteger seed(987654321 + std::time(nullptr));
- int32_t expected_value, new_value;
+uint32_t Object::GenerateIdentityHashCode() {
+ uint32_t expected_value, new_value;
do {
- expected_value = static_cast<uint32_t>(seed.LoadRelaxed());
+ expected_value = hash_code_seed.LoadRelaxed();
new_value = expected_value * 1103515245 + 12345;
- } while ((expected_value & LockWord::kHashMask) == 0 ||
- !seed.CompareExchangeWeakRelaxed(expected_value, new_value));
+ } while (!hash_code_seed.CompareExchangeWeakRelaxed(expected_value, new_value) ||
+ (expected_value & LockWord::kHashMask) == 0);
return expected_value & LockWord::kHashMask;
}
+void Object::SetHashCodeSeed(uint32_t new_seed) {
+ hash_code_seed.StoreRelaxed(new_seed);
+}
+
int32_t Object::IdentityHashCode() const {
mirror::Object* current_this = const_cast<mirror::Object*>(this);
while (true) {
diff --git a/runtime/mirror/object.h b/runtime/mirror/object.h
index ae1aeb5..bf76c86 100644
--- a/runtime/mirror/object.h
+++ b/runtime/mirror/object.h
@@ -343,6 +343,11 @@
void VisitReferences(const Visitor& visitor, const JavaLangRefVisitor& ref_visitor)
NO_THREAD_SAFETY_ANALYSIS;
+ // Used by object_test.
+ static void SetHashCodeSeed(uint32_t new_seed);
+ // Generate an identity hash code. Public for object test.
+ static uint32_t GenerateIdentityHashCode();
+
protected:
// Accessors for non-Java type fields
template<class T, VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags, bool kIsVolatile = false>
@@ -388,9 +393,6 @@
}
}
- // Generate an identity hash code.
- static int32_t GenerateIdentityHashCode();
-
// A utility function that copies an object in a read barrier and
// write barrier-aware way. This is internally used by Clone() and
// Class::CopyOf().
@@ -398,6 +400,8 @@
size_t num_bytes)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
+ static Atomic<uint32_t> hash_code_seed;
+
// The Class representing the type of the object.
HeapReference<Class> klass_;
// Monitor and hash code information.
diff --git a/runtime/mirror/object_test.cc b/runtime/mirror/object_test.cc
index 3e29e5f..1e6378f 100644
--- a/runtime/mirror/object_test.cc
+++ b/runtime/mirror/object_test.cc
@@ -716,5 +716,14 @@
// TODO: test that interfaces trump superclasses.
}
+TEST_F(ObjectTest, IdentityHashCode) {
+ // Regression test for b/19046417 which had an infinite loop if the
+ // (seed & LockWord::kHashMask) == 0. seed 0 triggered the infinite loop since we did the check
+ // before the CAS which resulted in the same seed the next loop iteration.
+ mirror::Object::SetHashCodeSeed(0);
+ int32_t hash_code = mirror::Object::GenerateIdentityHashCode();
+ EXPECT_NE(hash_code, 0);
+}
+
} // namespace mirror
} // namespace art