[utilities COMMIT] /java-support/trunk/src/main/java/net/shibboleth/utilities/java/support/collection/IndexingObjectS...

[noreply at shibboleth.net]

Author: putmanb
Date: Fri Jun  6 01:35:12 2014
New Revision: 585

URL: http://svn.shibboleth.net/view/utilities?rev=585&view=rev
Log:
Draft of fix for JSPT-39: IndexingObjectStore uses hashCode() in an unsafe manner.

Modified:
    java-support/trunk/src/main/java/net/shibboleth/utilities/java/support/collection/IndexingObjectStore.java

Modified: java-support/trunk/src/main/java/net/shibboleth/utilities/java/support/collection/IndexingObjectStore.java
URL: http://svn.shibboleth.net/view/utilities/java-support/trunk/src/main/java/net/shibboleth/utilities/java/support/collection/IndexingObjectStore.java?rev=585&r1=584&r2=585&view=diff
==============================================================================
--- java-support/trunk/src/main/java/net/shibboleth/utilities/java/support/collection/IndexingObjectStore.java (original)
+++ java-support/trunk/src/main/java/net/shibboleth/utilities/java/support/collection/IndexingObjectStore.java Fri Jun  6 01:35:12 2014
@@ -25,20 +25,29 @@
 import javax.annotation.concurrent.ThreadSafe;
 
 /**
- * This class is used to store instances of objects that may be created independently but are, in face, the same object.
+ * <p>
+ * This class is used to store instances of objects that may be created independently but are, in fact, the same object.
  * For example, KeyInfo XML structures contain keys, certs, and CRLs. Multiple unique instances of
  * a KeyInfo may contain, and separately construct, the exact same cert. KeyInfo could, therefore, create a class-level
  * instance of this object store and put certs within it. In this manner the cert is only sitting in memory once and
  * each KeyInfo simply stores a reference (index) to stored object.
+ * </p>
  * 
+ * <p>
  * This store uses basic reference counting to keep track of how many of the respective objects are pointing to an
- * entry. Adding an object that already exists, as determined by the objects <code>hashCode()</code> method, simply
+ * entry. Adding an object that already exists, as determined by the object's <code>equals()</code> method, simply
  * increments the reference counter. Removing an object decrements the counter. Only when the counter reaches zero is
  * the object actually freed for garbage collection.
+ * </p>
  * 
- * <strong>Note</strong> the instance of an object returned by {@link #get(String)} need not be the same object as 
- * stored via {@link #put(Object)}.  However, their hash codes will be equal.  Therefore this store should never be 
- * used to store objects that produce identical hash codes but are not functionally identical objects.
+ * <p>
+ * <strong>Note:</strong> the instance of an object returned by {@link #get(String)} need not be the same object as 
+ * stored via {@link #put(Object)}.  However, the instances will be equal according to their <code>equals()</code>.
+ * The indexing and storage is based on use of {@link Map}, so the normal caveats related to use of hash-based
+ * collection types apply: if the stored object's <code>hashCode()</code> and <code>equals()</code> methods are 
+ * implemented based on mutable properties of the object, then those object instance's properties should not 
+ * be mutated while the object is stored, otherwise unpredictable behavior will result.
+ * </p>
  * 
  * @param <T> type of object being stored
  */
@@ -50,11 +59,19 @@
 
     /** Backing object data store. */
     private Map<String, StoredObjectWrapper> objectStore;
+    
+    /** Map of object instances to the index value used to reference them externally. */
+    private Map<T, Integer> indexStore;
+    
+    /** The last index sequence used. */
+    private int lastIndex;
 
     /** Constructor. */
     public IndexingObjectStore() {
         rwLock = new ReentrantReadWriteLock();
         objectStore = new LazyMap<>();
+        indexStore = new LazyMap<>();
+        lastIndex = 0;
     }
 
     /** Clears the object store. */
@@ -63,6 +80,7 @@
         writeLock.lock();
         try {
             objectStore.clear();
+            indexStore.clear();
         } finally {
             writeLock.unlock();
         }
@@ -111,7 +129,7 @@
         Lock writeLock = rwLock.writeLock();
         writeLock.lock();
         try {
-            String index = Integer.toString(object.hashCode());
+            String index = getIndex(object);
 
             StoredObjectWrapper objectWrapper = objectStore.get(index);
             if (objectWrapper == null) {
@@ -172,6 +190,7 @@
                 objectWrapper.decremementReferenceCount();
                 if (objectWrapper.getReferenceCount() == 0) {
                     objectStore.remove(index);
+                    removeIndex(objectWrapper.getObject());
                 }
             }
         } finally {
@@ -187,6 +206,30 @@
      */
     public int size() {
         return objectStore.size();
+    }
+    
+    /**
+     * Get the index for the specified object.
+     * 
+     * @param object the target object
+     * @return the object index value
+     */
+    protected String getIndex(T object) {
+        Integer index = indexStore.get(object);

[... 18 lines stripped ...]