CODEWITHSUNDEEP
javalistconcurrentmodificationforeach
Bhesh Gurung
Bhesh Gurung

Reputation: 51030

Why am I not getting a java.util.ConcurrentModificationException in this example?

Note: I am aware of the Iterator#remove() method.

In the following code sample, I don't understand why the List.remove in main method throws ConcurrentModificationException, but not in the remove method.

public class RemoveListElementDemo {    
    private static final List<Integer> integerList;

    static {
        integerList = new ArrayList<Integer>();
        integerList.add(1);
        integerList.add(2);
        integerList.add(3);
    }

    public static void remove(Integer toRemove) {
        for(Integer integer : integerList) {
            if(integer.equals(toRemove)) {                
                integerList.remove(integer);
            }
        }
    }

    public static void main(String... args) {                
        remove(Integer.valueOf(2));

        Integer toRemove = Integer.valueOf(3);
        for(Integer integer : integerList) {
            if(integer.equals(toRemove)) {                
                integerList.remove(integer);
            }
        }
    }
}

Upvotes: 181

Views: 270284

Answers (10)

Stephen
Stephen

Reputation: 934

Change Iterator for each into for loop to solve.

And the Reason is:

The iterators returned by this class's iterator and listIterator methods are fail-fast: if the list is structurally modified at any time after the iterator is created, in any way except through the iterator's own remove or add methods, the iterator will throw a ConcurrentModificationException.

--Referred Java Docs.

Upvotes: 0

pushy
pushy

Reputation: 9645

Here's why: As it is says in the Javadoc:

The iterators returned by this class's iterator and listIterator methods are fail-fast: if the list is structurally modified at any time after the iterator is created, in any way except through the iterator's own remove or add methods, the iterator will throw a ConcurrentModificationException.

This check is done in the next() method of the iterator (as you can see by the stacktrace). But we will reach the next() method only if hasNext() delivered true, which is what is called by the for each to check if the boundary is met. In your remove method, when hasNext() checks if it needs to return another element, it will see that it returned two elements, and now after one element was removed the list only contains two elements. So all is peachy and we are done with iterating. The check for concurrent modifications does not occur, as this is done in the next() method which is never called.

Next we get to the second loop. After we remove the second number the hasNext method will check again if can return more values. It has returned two values already, but the list now only contains one. But the code here is:

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

1 != 2, so we continue to the next() method, which now realizes that someone has been messing with the list and fires the exception.

Hope that clears your question up.

Summary

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

Upvotes: 266

Gondil
Gondil

Reputation: 807

I had that same problem but in case that I was adding en element into iterated list. I made it this way

public static void remove(Integer remove) {
    for(int i=0; i<integerList.size(); i++) {
        //here is maybe fine to deal with integerList.get(i)==null
        if(integerList.get(i).equals(remove)) {                
            integerList.remove(i);
        }
    }
}

Now everything goes fine because you don't create any iterator over your list, you iterate over it "manually". And condition i < integerList.size() will never fool you because when you remove/add something into List size of the List decrement/increment..

Hope it helps, for me that was solution.

Upvotes: 4

JohnnyO
JohnnyO

Reputation: 547

If you use copy-on-write collections it will work; however when you use list.iterator(), the returned Iterator will always reference the collection of elements as it was when ( as below ) list.iterator() was called, even if another thread modifies the collection. Any mutating methods called on a copy-on-write–based Iterator or ListIterator (such as add, set, or remove) will throw an UnsupportedOperationException.

import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;

public class RemoveListElementDemo {    
    private static final List<Integer> integerList;

    static {
        integerList = new CopyOnWriteArrayList<>();
        integerList.add(1);
        integerList.add(2);
        integerList.add(3);
    }

    public static void remove(Integer remove) {
        for(Integer integer : integerList) {
            if(integer.equals(remove)) {                
                integerList.remove(integer);
            }
        }
    }

    public static void main(String... args) {                
        remove(Integer.valueOf(2));

        Integer remove = Integer.valueOf(3);
        for(Integer integer : integerList) {
            if(integer.equals(remove)) {                
                integerList.remove(integer);
            }
        }
    }
}

Upvotes: 1

Saif Hamed
Saif Hamed

Reputation: 1094

In my case I did it like this:

int cursor = 0;
do {
    if (integer.equals(remove))
        integerList.remove(cursor);
    else cursor++;
} while (cursor != integerList.size());

Upvotes: 0

RightHandedMonkey
RightHandedMonkey

Reputation: 1728

The forward/iterator method does not work when removing items. You can remove the element without error, but you will get a runtime error when you try to access removed items. You can't use the iterator because as pushy shows it will cause a ConcurrentModificationException, so use a regular for loop instead, but step backwards through it. 

List<Integer> integerList;
integerList = new ArrayList<Integer>();
integerList.add(1);
integerList.add(2);
integerList.add(3);

int size= integerList.size();

//Item to remove
Integer remove = Integer.valueOf(3);

A solution:

Traverse the array in reverse order if you are going to remove a list element. Simply by going backwards through the list you avoid visiting an item that has been removed, which removes the exception.

//To remove items from the list, start from the end and go backwards through the arrayList
//This way if we remove one from the beginning as we go through, then we will avoid getting a runtime error
//for java.lang.IndexOutOfBoundsException or java.util.ConcurrentModificationException as when we used the iterator
for (int i=size-1; i> -1; i--) {
    if (integerList.get(i).equals(remove) ) {
        integerList.remove(i);
    }
}

Upvotes: 15

Abhishek
Abhishek

Reputation: 55

Check your code man....

In the main method you are trying to remove the 4th element which is not there and hence the error. In the remove() method you are trying to remove the 3rd element which is there and hence no error.

Upvotes: -2

Saurabh
Saurabh

Reputation: 91

This runs fine on Java 1.6

~ % javac RemoveListElementDemo.java
~ % java RemoveListElementDemo
~ % cat RemoveListElementDemo.java 

import java.util.*;
public class RemoveListElementDemo {    
    private static final List<Integer> integerList;

    static {
        integerList = new ArrayList<Integer>();
        integerList.add(1);
        integerList.add(2);
        integerList.add(3);
    }

    public static void remove(Integer remove) {
        for(Integer integer : integerList) {
            if(integer.equals(remove)) {                
                integerList.remove(integer);
            }
        }
    }

    public static void main(String... args) {                
        remove(Integer.valueOf(2));

        Integer remove = Integer.valueOf(3);
        for(Integer integer : integerList) {
            if(integer.equals(remove)) {                
                integerList.remove(integer);
            }
        }
    }
}

~ %

Upvotes: 0

James Raitsev
James Raitsev

Reputation: 96531

One way to handle it it to remove something from a copy of a Collection (not Collection itself), if applicable. Clone the original collection it to make a copy via a Constructor.

This exception may be thrown by methods that have detected concurrent modification of an object when such modification is not permissible.

For your specific case, first off, i don't think final is a way to go considering you intend to modify the list past declaration

private static final List<Integer> integerList;

Also consider modifying a copy instead of the original list.

List<Integer> copy = new ArrayList<Integer>(integerList);

for(Integer integer : integerList) {
    if(integer.equals(remove)) {                
        copy.remove(integer);
    }
}

Upvotes: 43

Sachin
Sachin

Reputation: 18777

This snippet will always throw a ConcurrentModificationException.

The rule is "You may not modify (add or remove elements from the list) while iterating over it using an Iterator (which happens when you use a for-each loop)".

JavaDocs:

The iterators returned by this class's iterator and listIterator methods are fail-fast: if the list is structurally modified at any time after the iterator is created, in any way except through the iterator's own remove or add methods, the iterator will throw a ConcurrentModificationException.

Hence if you want to modify the list (or any collection in general), use iterator, because then it is aware of the modifications and hence those will be handled properly.

Hope this helps.

Upvotes: 8

Related Questions