LANG-459 HashCodeBuilder.reflectionHashCode() can generate incorrect hashcodes

git-svn-id: https://svn.apache.org/repos/asf/commons/proper/lang/trunk@695951 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/commons/lang/IDKey.java

@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * 
+ */
+
+package org.apache.commons.lang;
+
+// adapted from org.apache.axis.utils.IDKey
+
+/**
+* Wrap an identity key (System.identityHashCode()) 
+* so that an object can only be equal() to itself.
+* 
+* This is necessary to disambiguate the occasional duplicate
+* identityHashCodes that can occur.
+*  
+*/ 
+public final class IDKey {
+        private final Object value;
+        private final int id;
+
+        /**
+         * Constructor for IDKey
+         * @param _value
+         */ 
+        public IDKey(Object _value) {
+            // This is the Object hashcode 
+            id = System.identityHashCode(_value);  
+            // There have been some cases (LANG-459) that return the 
+            // same identity hash code for different objects.  So 
+            // the value is also added to disambiguate these cases.
+            value = _value;
+        }
+
+        /**
+         * returns hashcode - i.e. the system identity hashcode.
+         * @return the hashcode
+         */ 
+        public int hashCode() {
+           return id;
+        }
+
+        /**
+         * checks if instances are equal
+         * @param other
+         * @return if the instances are for the same object
+         */ 
+        public boolean equals(Object other) {
+            if (!(other instanceof IDKey)) {
+                return false;
+            }
+            IDKey idKey = (IDKey) other;
+            if (id != idKey.id) {
+                return false;
+            }
+            // Note that identity equals is used.
+            return value == idKey.value;
+         }
+}

src/java/org/apache/commons/lang/builder/HashCodeBuilder.java

@@ -27,6 +27,8 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
+import org.apache.commons.lang.IDKey;
+
 /**
  * <p>
  * Assists in implementing {@link Object#hashCode()} methods.
@@ -110,6 +112,23 @@ public class HashCodeBuilder {
         }
     };
 
+    /*
+     * N.B. we cannot store the actual objects in a HashSet, as that would use the very hashCode()
+     * we are in the process of calculating.
+     * 
+     * So we generate a one-to-one mapping from the original object to a new object.
+     * 
+     * Now HashSet uses equals() to determine if two elements with the same hashcode really
+     * are equal, so we also need to ensure that the replacement objects are only equal
+     * if the original objects are identical.
+     * 
+     * The original implementation (2.4 and before) used the System.indentityHashCode()
+     * method - however this is not guaranteed to generate unique ids (e.g. LANG-459)
+     *  
+     * We now use the IDKey helper class (adapted from org.apache.axis.utils.IDKey)
+     * to disambiguate the duplicate ids.
+     */
+    
     /**
      * <p>
      * Returns the registry of objects being traversed by the reflection methods in the current thread.
@@ -134,7 +153,7 @@ public class HashCodeBuilder {
      * @since 2.3
      */
     static boolean isRegistered(Object value) {
-        return getRegistry().contains(toIdentityHashCodeInteger(value));
+        return getRegistry().contains(new IDKey(value));
     }
 
     /**
@@ -506,19 +525,7 @@ public class HashCodeBuilder {
      *            The object to register.
      */
     static void register(Object value) {
-        getRegistry().add(toIdentityHashCodeInteger(value));
+        getRegistry().add(new IDKey(value));
-    }
-
-    /**
-     * Returns an Integer for the given object's default hash code.
-     * 
-     * @see System#identityHashCode(Object)
-     * @param value
-     *            object for which the hashCode is to be calculated
-     * @return Default int hash code
-     */
-    private static Integer toIdentityHashCodeInteger(Object value) {
-        return new Integer(System.identityHashCode(value));
     }
 
     /**
@@ -534,7 +541,7 @@ public class HashCodeBuilder {
      * @since 2.3
      */
     static void unregister(Object value) {
-        getRegistry().remove(toIdentityHashCodeInteger(value));
+        getRegistry().remove(new IDKey(value));
     }
 
     /**