JDK-8169679 : ArrayList.subList().iterator().forEachRemaining() off-by-one-error
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.util:collections
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2016-11-14
  • Updated: 2016-12-08
  • Resolved: 2016-11-29
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 9
9 b148Fixed
Description
If ArrayList subList iterator forEachRemaining() is called, lastRet is incorrectly set to cursor, not to cursor - 1 as in another forEachRemaining call in the same source file.

Discovered by adding subList tests to ArrayListTest:

Index: src/test/tck/ArrayListTest.java
===================================================================
RCS file: /export/home/jsr166/jsr166/jsr166/src/test/tck/ArrayListTest.java,v
retrieving revision 1.1
diff -u -r1.1 ArrayListTest.java
--- src/test/tck/ArrayListTest.java	17 Oct 2016 17:52:30 -0000	1.1
+++ src/test/tck/ArrayListTest.java	14 Nov 2016 21:05:16 -0000
@@ -7,6 +7,7 @@
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.List;
 
 import junit.framework.Test;
 import junit.framework.TestSuite;
@@ -19,13 +20,20 @@
     public static Test suite() {
         class Implementation implements CollectionImplementation {
             public Class<?> klazz() { return ArrayList.class; }
-            public Collection emptyCollection() { return new ArrayList(); }
+            public List emptyCollection() { return new ArrayList(); }
             public Object makeElement(int i) { return i; }
             public boolean isConcurrent() { return false; }
             public boolean permitsNulls() { return true; }
         }
-        return newTestSuite(// ArrayListTest.class,
-                            CollectionTest.testSuite(new Implementation()));
+        class SubListImplementation extends Implementation {
+            public List emptyCollection() {
+                return super.emptyCollection().subList(0, 0);
+            }
+        }
+        return newTestSuite(
+                // ArrayListTest.class,
+                CollectionTest.testSuite(new Implementation()),
+                CollectionTest.testSuite(new SubListImplementation()));
     }
 
 }

giving:

     [java] 1) testRemoveAfterForEachRemaining(Collection8Test)java.util.ConcurrentModificationException
     [java] 	at java.base/java.util.ArrayList$SubList$1.remove(ArrayList.java:1233)
     [java] 	at Collection8Test.testRemoveAfterForEachRemaining(Collection8Test.java:403)
     [java] 	at jdk.internal.reflect.GeneratedMethodAccessor4.invoke(Unknown Source)
     [java] 	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
     [java] 	at JSR166TestCase.runTest(JSR166TestCase.java:303)
     [java] 	at JSR166TestCase.runBare(JSR166TestCase.java:294)
     [java] 	at JSR166TestCase.main(JSR166TestCase.java:367)
     [java] 	at ArrayListTest.main(ArrayListTest.java:17)

Comments
The simplest fix is to align the two forEachRemaining methods: Index: src/main/java/util/ArrayList.java =================================================================== RCS file: /export/home/jsr166/jsr166/jsr166/src/main/java/util/ArrayList.java,v retrieving revision 1.41 diff -u -r1.41 ArrayList.java --- src/main/java/util/ArrayList.java 13 Nov 2016 21:07:40 -0000 1.41 +++ src/main/java/util/ArrayList.java 14 Nov 2016 21:13:23 -0000 @@ -1207,7 +1207,8 @@ consumer.accept((E) elementData[offset + (i++)]); } // update once at end of iteration to reduce heap write traffic - lastRet = cursor = i; + cursor = i; + lastRet = i - 1; checkForComodification(); }
14-11-2016