Weird Java Concurrent modification exception example

Question

If we write like this, there is a concurrent modification exception :

public static void main(String... args) {
    List<String> listOfBooks = new ArrayList<>();
    listOfBooks.add("Programming Pearls");
    listOfBooks.add("Clean Code");
    listOfBooks.add("Effective Java");
    listOfBooks.add("Code Complete");

    System.err.println("Before deleting : " + listOfBooks);
    for (String book : listOfBooks) {
        if (book.contains("Code")) {
            listOfBooks.remove(book);
        }
    }
    System.err.println("After deleting : " + listOfBooks);
}

On the other hand, if we write like this, there is NO concurrent modification exception ! Notice that code is exact the same, except the strings for compare, in first example it is a Code, and in second it is a Java

public static void main(String... args) {
    List<String> listOfBooks = new ArrayList<>();
    listOfBooks.add("Programming Pearls");
    listOfBooks.add("Clean Code");
    listOfBooks.add("Effective Java");
    listOfBooks.add("Code Complete");

    System.err.println("Before deleting : " + listOfBooks);
    for (String book : listOfBooks) {
        if (book.contains("Java")) {
            listOfBooks.remove(book);
        }
    }
    System.err.println("After deleting : " + listOfBooks);
}

I'm using Netbeans 8.2, Windows 7 32bit, with JDK 1.8.0_131 What's wrong ?

Answer 1

List.remove() will not throw ConcurrentModificationException when it removes the second last element from the list.

Quoting from this Java Bug (JDK-4902078) .

When the Collections Framework was added to the platform it was deemed too expensive to check for comodification once rather than twice per iteration; the check was made on Iterator.next rather than Iterator.hasNext. Expert reviewers thought this was sufficient. They were unaware that it fails to detect one important case: if an element is removed from the list immediately prior to the final call to hasNext in an iteration, the call returns false and the iteration terminates, silently ignoring the last element on the list.

You can also check this answer :-

https://stackoverflow.com/a/8189786/1992276

Answer 2

There are two ways used to iterate over an collection: enumeration and iterator.

First one allows for the collection to be modified during iteration (fail slow), second does not (fail fast). In a for-each loop you are using an iterator, so any modification to the collection, during it's iteration would cause an exception.

You have 3 choices, to solve this problem:

Use an iterator instead:

Iterator<String> bookIt = listOfBooks.iterator();
while(bookIt.hasNext()){
   String book = bookIt.next();
   if (book.contains("Java")) {
       bookIt.remove();
   }
}

Create a new list with only acceptable elements (filter out the unwanted):

 List<String> booksWithNoCode =  listOfBooks.stream()
 .filter(book-> !book.contains("Code"))
 .collect(toList())

Use Collection.removeIf(), you will remove all elements from the list, that are matching given criteria.

listOfBooks.removeIf(book-> book.contains("Code"))

You can find more information in this post and here.

Answer 3

You can't modify the listOfBooks while you are iterating though it with the for each loop.

---

edit:

for (String book : listOfBooks) {
    if (book.contains("Code")) {
        listOfBooks.remove(book);
    }
}

Is the same as:

    for (Iterator<String> i = listOfBooks.iterator(); i.hasNext();) {
        String book = i.next();
        if (book.contains("Code")) {
            listOfBooks.remove(book);
        }
    }

http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/tip/src/share/classes/java/util/ArrayList.java

The key in the arraylist code is:

public boolean remove(Object o) {
    if (o == null) {
        for (int index = 0; index < size; index++)
            if (elementData[index] == null) {
                fastRemove(index);
                return true;
            }
    } else {
        for (int index = 0; index < size; index++)
            if (o.equals(elementData[index])) {
                fastRemove(index);
                return true;
            }
    }
    return false;
}

/*
 * Private remove method that skips bounds checking and does not
 * return the value removed.
 */
private void fastRemove(int index) {
    modCount++;
    int numMoved = size - index - 1;
    if (numMoved > 0)
        System.arraycopy(elementData, index+1, elementData, index,
                         numMoved);
    elementData[--size] = null; // clear to let GC do its work
}

and the iterator code:

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

    @SuppressWarnings("unchecked")
    public E next() {
        checkForComodification();
        int i = cursor;
        if (i >= size)
            throw new NoSuchElementException();
        Object[] elementData = ArrayList.this.elementData;
        if (i >= elementData.length)
            throw new ConcurrentModificationException();
        cursor = i + 1;
        return (E) elementData[lastRet = i];
    }

The cursor always points to the next element so when you get the "Effective Java" i = 2 but cursor is 3.

When you call the remove the cursor is at 3 and the size is 4.

The size is then decremented by the remove and now cursor == size and the next hasNext() returns false ending the loop.