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
Change-Id: I7d1c377dd1bda780681514b24d61ebc776bc80ab
diff --git a/runtime/mirror/object.cc b/runtime/mirror/object.cc
index 8bccd9e..6914f38 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)
@@ -136,16 +138,19 @@
}
uint32_t Object::GenerateIdentityHashCode() {
- static Atomic<uint32_t> seed(987654321U + std::time(nullptr));
uint32_t expected_value, new_value;
do {
- expected_value = 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 221feca..07d15b5 100644
--- a/runtime/mirror/object.h
+++ b/runtime/mirror/object.h
@@ -419,6 +419,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>
@@ -471,9 +476,6 @@
}
}
- // Generate an identity hash code.
- static uint32_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().
@@ -481,6 +483,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 f9c00ce..fb42d28 100644
--- a/runtime/mirror/object_test.cc
+++ b/runtime/mirror/object_test.cc
@@ -731,5 +731,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
