You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2018/02/01 18:50:18 UTC

commons-collections git commit: [COLLECTIONS-599] HashEntry array object naming data initialized with double the size during deserialization.

Repository: commons-collections
Updated Branches:
  refs/heads/master 1e6435ec1 -> 4a9bcd0f5


[COLLECTIONS-599] HashEntry array object naming data initialized with
double the size during deserialization.

Project: http://git-wip-us.apache.org/repos/asf/commons-collections/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-collections/commit/4a9bcd0f
Tree: http://git-wip-us.apache.org/repos/asf/commons-collections/tree/4a9bcd0f
Diff: http://git-wip-us.apache.org/repos/asf/commons-collections/diff/4a9bcd0f

Branch: refs/heads/master
Commit: 4a9bcd0f5211c91c4c2f47313c8f8013d0dfe03e
Parents: 1e6435e
Author: Gary Gregory <gg...@apache.org>
Authored: Thu Feb 1 11:50:15 2018 -0700
Committer: Gary Gregory <gg...@apache.org>
Committed: Thu Feb 1 11:50:15 2018 -0700

----------------------------------------------------------------------
 src/changes/changes.xml                         |  5 +++-
 .../collections4/map/AbstractReferenceMap.java  | 10 ++++++-
 .../collections4/map/ReferenceMapTest.java      | 31 ++++++++++++++++++++
 3 files changed, 44 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/commons-collections/blob/4a9bcd0f/src/changes/changes.xml
----------------------------------------------------------------------
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index d2ae55e..85f0f96 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -20,7 +20,10 @@
     <title>Commons Collections Changes</title>
   </properties>
   <body>
-  <release version="4.2" date="YYYY-MM-DD" description="Update from Java 6 to Java 7 and small changes.">
+  <release version="4.2" date="2018-MM-DD" description="Update from Java 6 to Java 7, bug fixes, and small changes.">
+    <action issue="COLLECTIONS-599" dev="ggregory" type="fix" due-to="Tejas Patel, Saleem Akbar, Gary Gregory">
+      HashEntry array object naming data initialized with double the size during deserialization.
+    </action>
     <action issue="COLLECTIONS-662" dev="chtompki" type="fix" due-to="Vamsi Kavuri">
       Unit tests MapUtilsTest and ListIteratorWrapperTest no longer fail on Java 9.
     </action>

http://git-wip-us.apache.org/repos/asf/commons-collections/blob/4a9bcd0f/src/main/java/org/apache/commons/collections4/map/AbstractReferenceMap.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/collections4/map/AbstractReferenceMap.java b/src/main/java/org/apache/commons/collections4/map/AbstractReferenceMap.java
index f114083..e0fa7e2 100644
--- a/src/main/java/org/apache/commons/collections4/map/AbstractReferenceMap.java
+++ b/src/main/java/org/apache/commons/collections4/map/AbstractReferenceMap.java
@@ -1045,6 +1045,15 @@ public abstract class AbstractReferenceMap<K, V> extends AbstractHashedMap<K, V>
         final int capacity = in.readInt();
         init();
         data = new HashEntry[capacity];
+
+        // COLLECTIONS-599: Calculate threshold before populating, otherwise it will be 0 
+        // when it hits AbstractHashedMap.checkCapacity() and so will unnecessarily 
+        // double up the size of the "data" array during population.
+        //
+        // NB: AbstractHashedMap.doReadObject() DOES calculate the threshold before populating.
+        //
+        threshold = calculateThreshold(data.length, loadFactor);
+        
         while (true) {
             final K key = (K) in.readObject();
             if (key == null) {
@@ -1053,7 +1062,6 @@ public abstract class AbstractReferenceMap<K, V> extends AbstractHashedMap<K, V>
             final V value = (V) in.readObject();
             put(key, value);
         }
-        threshold = calculateThreshold(data.length, loadFactor);
         // do not call super.doReadObject() as code there doesn't work for reference map
     }
 

http://git-wip-us.apache.org/repos/asf/commons-collections/blob/4a9bcd0f/src/test/java/org/apache/commons/collections4/map/ReferenceMapTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/collections4/map/ReferenceMapTest.java b/src/test/java/org/apache/commons/collections4/map/ReferenceMapTest.java
index 4ce341f..3ea05e0 100644
--- a/src/test/java/org/apache/commons/collections4/map/ReferenceMapTest.java
+++ b/src/test/java/org/apache/commons/collections4/map/ReferenceMapTest.java
@@ -16,6 +16,12 @@
  */
 package org.apache.commons.collections4.map;
 
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+
 import java.lang.ref.WeakReference;
 import java.util.Map;
 
@@ -249,6 +255,31 @@ public class ReferenceMapTest<K, V> extends AbstractIterableMapTest<K, V> {
         }
     }
 
+    /**
+     * Test whether after serialization the "data" HashEntry array is the same size as the original.<p>
+     * 
+     * See <a href="https://issues.apache.org/jira/browse/COLLECTIONS-599">COLLECTIONS-599: HashEntry array object naming data initialized with double the size during deserialization</a>
+     */
+    public void testDataSizeAfterSerialization() throws IOException, ClassNotFoundException {
+        
+        ReferenceMap<String,String> serialiseMap = new ReferenceMap<>(ReferenceStrength.WEAK, ReferenceStrength.WEAK, true);
+        serialiseMap.put("KEY", "VALUE");
+        
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        try (ObjectOutputStream out = new ObjectOutputStream(baos)) {
+            out.writeObject(serialiseMap);
+        }
+        
+        ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray());
+        try (ObjectInputStream in = new ObjectInputStream(bais)) {
+            @SuppressWarnings("unchecked")
+            ReferenceMap<String,String> deserialisedMap = (ReferenceMap<String,String>) in.readObject();
+            assertEquals(1, deserialisedMap.size());
+            assertEquals(serialiseMap.data.length, deserialisedMap.data.length);
+        }
+        
+    }
+    
     @SuppressWarnings("unused")
     private static void gc() {
         try {