8013649: HashMap spliterator tryAdvance() encounters remaining elements after forEachRemaining()
authorpsandoz
Fri, 31 May 2013 10:53:19 +0200
changeset 17949 6c33d8f2601e
parent 17948 de14cee4b93d
child 17950 b2d5b298ec6e
8013649: HashMap spliterator tryAdvance() encounters remaining elements after forEachRemaining() Reviewed-by: chegar
jdk/src/share/classes/java/util/HashMap.java
jdk/src/share/classes/java/util/WeakHashMap.java
jdk/test/java/util/Spliterator/SpliteratorTraversingAndSplittingTest.java
--- a/jdk/src/share/classes/java/util/HashMap.java	Wed Jun 05 11:12:31 2013 +0100
+++ b/jdk/src/share/classes/java/util/HashMap.java	Fri May 31 10:53:19 2013 +0200
@@ -2701,8 +2701,10 @@
                     action.accept(m.nullKeyEntry.key);
                 }
             }
-            if (tab.length >= hi && (i = index) >= 0 && i < (index = hi)) {
+            if (tab.length >= hi && (i = index) >= 0 &&
+                (i < (index = hi) || current != null)) {
                 Object p = current;
+                current = null;
                 do {
                     if (p == null) {
                         p = tab[i++];
@@ -2815,8 +2817,10 @@
                     action.accept(m.nullKeyEntry.value);
                 }
             }
-            if (tab.length >= hi && (i = index) >= 0 && i < (index = hi)) {
+            if (tab.length >= hi && (i = index) >= 0 &&
+                (i < (index = hi) || current != null)) {
                 Object p = current;
+                current = null;
                 do {
                     if (p == null) {
                         p = tab[i++];
@@ -2928,8 +2932,10 @@
                     action.accept(m.nullKeyEntry);
                 }
             }
-            if (tab.length >= hi && (i = index) >= 0 && i < (index = hi)) {
+            if (tab.length >= hi && (i = index) >= 0 &&
+                (i < (index = hi) || current != null)) {
                 Object p = current;
+                current = null;
                 do {
                     if (p == null) {
                         p = tab[i++];
--- a/jdk/src/share/classes/java/util/WeakHashMap.java	Wed Jun 05 11:12:31 2013 +0100
+++ b/jdk/src/share/classes/java/util/WeakHashMap.java	Fri May 31 10:53:19 2013 +0200
@@ -1100,9 +1100,10 @@
             }
             else
                 mc = expectedModCount;
-            if (tab.length >= hi && (i = index) >= 0 && i < hi) {
-                index = hi;
+            if (tab.length >= hi && (i = index) >= 0 &&
+                (i < (index = hi) || current != null)) {
                 WeakHashMap.Entry<K,V> p = current;
+                current = null; // exhaust
                 do {
                     if (p == null)
                         p = tab[i++];
@@ -1179,9 +1180,10 @@
             }
             else
                 mc = expectedModCount;
-            if (tab.length >= hi && (i = index) >= 0 && i < hi) {
-                index = hi;
+            if (tab.length >= hi && (i = index) >= 0 &&
+                (i < (index = hi) || current != null)) {
                 WeakHashMap.Entry<K,V> p = current;
+                current = null; // exhaust
                 do {
                     if (p == null)
                         p = tab[i++];
@@ -1256,9 +1258,10 @@
             }
             else
                 mc = expectedModCount;
-            if (tab.length >= hi && (i = index) >= 0 && i < hi) {
-                index = hi;
+            if (tab.length >= hi && (i = index) >= 0 &&
+                (i < (index = hi) || current != null)) {
                 WeakHashMap.Entry<K,V> p = current;
+                current = null; // exhaust
                 do {
                     if (p == null)
                         p = tab[i++];
--- a/jdk/test/java/util/Spliterator/SpliteratorTraversingAndSplittingTest.java	Wed Jun 05 11:12:31 2013 +0100
+++ b/jdk/test/java/util/Spliterator/SpliteratorTraversingAndSplittingTest.java	Fri May 31 10:53:19 2013 +0200
@@ -128,6 +128,10 @@
 
         void addMap(Function<Map<T, T>, ? extends Map<T, T>> m) {
             String description = "new " + m.apply(Collections.<T, T>emptyMap()).getClass().getName();
+            addMap(m, description);
+        }
+
+        void addMap(Function<Map<T, T>, ? extends Map<T, T>> m, String description) {
             add(description + ".keySet().spliterator()", () -> m.apply(mExp).keySet().spliterator());
             add(description + ".values().spliterator()", () -> m.apply(mExp).values().spliterator());
             add(description + ".entrySet().spliterator()", mExp.entrySet(), () -> m.apply(mExp).entrySet().spliterator());
@@ -399,12 +403,36 @@
 
             db.addMap(HashMap::new);
 
+            db.addMap(m -> {
+                // Create a Map ensuring that for large sizes
+                // buckets will contain 2 or more entries
+                HashMap<Integer, Integer> cm = new HashMap<>(1, m.size() + 1);
+                // Don't use putAll which inflates the table by
+                // m.size() * loadFactor, thus creating a very sparse
+                // map for 1000 entries defeating the purpose of this test,
+                // in addition it will cause the split until null test to fail
+                // because the number of valid splits is larger than the
+                // threshold
+                for (Map.Entry<Integer, Integer> e : m.entrySet())
+                    cm.put(e.getKey(), e.getValue());
+                return cm;
+            }, "new java.util.HashMap(1, size + 1)");
+
             db.addMap(LinkedHashMap::new);
 
             db.addMap(IdentityHashMap::new);
 
             db.addMap(WeakHashMap::new);
 
+            db.addMap(m -> {
+                // Create a Map ensuring that for large sizes
+                // buckets will be consist of 2 or more entries
+                WeakHashMap<Integer, Integer> cm = new WeakHashMap<>(1, m.size() + 1);
+                for (Map.Entry<Integer, Integer> e : m.entrySet())
+                    cm.put(e.getKey(), e.getValue());
+                return cm;
+            }, "new java.util.WeakHashMap(1, size + 1)");
+
             // @@@  Descending maps etc
             db.addMap(TreeMap::new);