From b34d505b090cfefd01665aa735364962a364303e Mon Sep 17 00:00:00 2001
From: "tony.ys_liu" <tony.ys_liu@htc.com>
Date: Fri, 16 Jan 2015 19:16:45 +0800
Subject: [PATCH] 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
---
 runtime/mirror/object.cc      | 17 +++++++++++------
 runtime/mirror/object.h       | 10 +++++++---
 runtime/mirror/object_test.cc |  9 +++++++++
 3 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/runtime/mirror/object.cc b/runtime/mirror/object.cc
index 07bdf4387..d2cc367d4 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 @@ Object* Object::Clone(Thread* self) {
   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 ae1aeb5ae..bf76c860e 100644
--- a/runtime/mirror/object.h
+++ b/runtime/mirror/object.h
@@ -343,6 +343,11 @@ class MANAGED LOCKABLE Object {
   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 @@ class MANAGED LOCKABLE Object {
     }
   }
 
-  // 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 @@ class MANAGED LOCKABLE Object {
                             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 3e29e5f7c..1e6378f4d 100644
--- a/runtime/mirror/object_test.cc
+++ b/runtime/mirror/object_test.cc
@@ -716,5 +716,14 @@ TEST_F(ObjectTest, FindStaticField) {
   // 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
-- 
2.11.0