10

I have the below code and I would expect it to throw a ConcurrentModificationException, but it runs successfully. Why does this happen?

public void fun(){
    List <Integer>lis = new ArrayList<Integer>();
    lis.add(1);
    lis.add(2);

    for(Integer st:lis){
        lis.remove(1);
        System.out.println(lis.size());
    }
}

public static void main(String[] args) {
    test t = new test();
    t.fun();
}
Jason Nichols
  • 11,603
  • 5
  • 34
  • 53
Alok Pathak
  • 875
  • 1
  • 8
  • 20
  • Why would it throw that error? [ArrayList.remove()](http://docs.oracle.com/javase/7/docs/api/java/util/ArrayList.html#remove(int)) doesn't throw that error, only index out of bounds. – SyntaxTerror Jul 03 '14 at 14:32
  • 1
    psssst!.... you never saw my answer to the question :) – Oliver Watkins Jul 03 '14 at 14:37
  • Please find java core team explanation on this here https://bugs.java.com/bugdatabase/view_bug?bug_id=4902078 – sankar Mar 30 '23 at 11:52

7 Answers7

10

The remove(int) method on List removes the element at the specified position. Before you start your loop, your list looks like this:

[1, 2]

Then you start an iterator on the list:

[1, 2]
 ^

Your for loop then removes the element at position 1, which is the number 2:

[1]
 ^

The iterator, on the next implied hasNext() call, returns false, and the loop terminates.

You will get a ConcurrentModificationException if you add more elements to the list. Then the implicit next() will throw.

As a note, from the Javadoc for ArrayList from the JCF:

Note that the fail-fast behavior of an iterator cannot be guaranteed as it is, generally speaking, impossible to make any hard guarantees in the presence of unsynchronized concurrent modification. Fail-fast iterators throw ConcurrentModificationException on a best-effort basis. Therefore, it would be wrong to write a program that depended on this exception for its correctness: the fail-fast behavior of iterators should be used only to detect bugs.

This is probably actually a bug in the Oracle ArrayList iterator implementation; hasNext() does not check for modification:

public boolean hasNext() {
    return cursor != size;
}
chrylis -cautiouslyoptimistic-
  • 75,269
  • 21
  • 115
  • 152
3

It doesn't throw a ConcurrentModificationException because, as vandale said, the iterator only checks for comodification on next(). Here's a portion of the Iterator instance returned by an ArrayList:

    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];
    }

hasNext() simply looks to see if the cursor is pointing at the last index of the list. It doesn't check to see if the list was modified. Because of this you don't get an ConcurrentModificationException, it just stops iterating.

Jason Nichols
  • 11,603
  • 5
  • 34
  • 53
1

if you have a list of 3 like:

lis.add(1);
lis.add(2);
lis.add(3);

you will get ConcurrentModificationException in your case. PS: I have tried this!

Bill Lin
  • 1,145
  • 1
  • 11
  • 12
1

Because you aren't removing 1, you are removing the element at 1. ( remove(int) vs remove(Object) )

The iterator will only check for modification on a call to next() and not hasNext(), and the loop will exit after the call to hasNext() because you have removed 2, the list is only one long and thus exits.

vandale
  • 3,600
  • 3
  • 22
  • 39
  • 2
    Actually, if you're removing at index 0 it also fails to throw an exception. Please test your answers before posting them. – Ordous Jul 03 '14 at 14:40
  • 1
    @Ordous it is by the same principle, that the loop will exit before it checks to see if the list has been modified – vandale Jul 03 '14 at 14:44
  • 1
    Same principle, but the first sentence is completely irrelevant, and the "Because" throws anyone reading it off the actual reason. – Ordous Jul 03 '14 at 14:50
1

The gist of the matter is, as stated on both ArrayList and ConcurrentModificationException:

Note that the fail-fast behavior of an iterator cannot be guaranteed as it is, generally speaking, impossible to make any hard guarantees in the presence of unsynchronized concurrent modification. Fail-fast iterators throw ConcurrentModificationException on a best-effort basis.

Now a code sample from the Iterator returned by ArrayList:

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

    public E next() {
        checkForComodification();
        <stuff>
        return <things>;
    }

    <more methods>

    final void checkForComodification() {
        if (modCount != expectedModCount)
            throw new ConcurrentModificationException();
    }

As you can clearly see, in the case of ArrayList the "best effort" is in checking for modifications when calling next() and not when calling getNext(). Your loop terminates without calling next() a second time, hence no exception. Should you have 3 elements to begin with, or add an element it will fail. Also worth noting that if you modify the array list using reflection without updating the modCount variable (naughty...), then the exception will not be thrown at all. modCount is also not volatile, which again shows it's only best effort and has no guarantees, since the iterator might not see the latest value anyway.

Ordous
  • 3,844
  • 15
  • 25
0

In this loop:

   for(Integer st:lis){
        lis.remove(1);
        System.out.println(lis.size());
    }

You are only constantly removing element with index 1 from the matrix without even caring what is in st. So this loop and with every iteration will try to remove item with index 1. Concurent modification will come up with tthis loop:

 for(Integer st:lis){
    lis.remove(st);
    System.out.println(lis.size());
}
TheMP
  • 8,257
  • 9
  • 44
  • 73
  • 2
    This is actually incorrect, as he's removing the item from index 1. It would only remove the value of 1 if Java is autoboxing the int 1 to an Integer 1. – Jason Nichols Jul 03 '14 at 14:34
  • 1
    You are correct and I was dumb. Can't believe that this answer was upvoted twice. I will try to fix it. – TheMP Jul 03 '14 at 14:39
0

You have only 2 entries in the list. So, the loop runs only once since you are removing an entry.

ConcurrentModificationException will be thrown if a list is modified and again you try to do some operation on it. But before doing any operation on it, we are out of the loop hence no exception. Try adding another entry into list and run your program which will throw exception.

vinayknl
  • 1,242
  • 8
  • 18