Re: Bug in ArrayList iterator

[Chris Hegarty]

On 07/01/15 10:47, Daniel Fuchs wrote:
On 07/01/15 11:31, Paul Sandoz wrote:

On Jan 7, 2015, at 10:45 AM, Remi Forax <fo...@univ-mlv.fr> wrote:

A simple Java question, what this code does ?

  ArrayList<String> list = new ArrayList<>();
  list.add("foo");
  list.add("bar");
  for(String s: list) {
    list.remove(s);
  }

:(


We could improve the best-effort basis by which
ConcurrentModificationException is thrown by snapshotting the
collection size on construction of the iterator

This was my thought too.

  diff --git a/src/java.base/share/classes/java/util/ArrayList.java 
b/src/java.base/share/classes/java/util/ArrayList.java
--- a/src/java.base/share/classes/java/util/ArrayList.java
+++ b/src/java.base/share/classes/java/util/ArrayList.java
@@ -853,9 +853,10 @@
         int cursor;       // index of next element to return
         int lastRet = -1; // index of last element returned; -1 if no such
         int expectedModCount = modCount;
+        int itrSize = size;

         public boolean hasNext() {
-            return cursor != size;
+            return cursor != itrSize;
         }

         @SuppressWarnings("unchecked")
@@ -881,6 +882,7 @@
                 cursor = lastRet;
                 lastRet = -1;
                 expectedModCount = modCount;
+                itrSize--;
             } catch (IndexOutOfBoundsException ex) {
                 throw new ConcurrentModificationException();
             }
@@ -909,7 +911,7 @@
         }

         final void checkForComodification() {
-            if (modCount != expectedModCount)
+            if (modCount != expectedModCount || size != itrSize)
                 throw new ConcurrentModificationException();
         }
     }


>> (at the expense of an
extra field, but that might pack into a spare 4 bytes due to alignment
at least on 64 bits IIUC).

Or call checkForComodification() when cursor == size in hasNext()?


I'm not sure that we'd want hasNext to possibly throw a CME. I think it 
best that next do that, if needed.

-Chris.

-- daniel


Paul.

Rémi
tip: the bug lies in ArrayList.Itr.hasNext() (and
AbstractList.Itr.hasNext()).