Concurrent Modification Exception in Java

Question

I am getting the ConcurrentModificationException while executing this code. I am unable to figure out why it is happening?

private void verifyBookingIfAvailable(ArrayList<Integer> list, int id) {

        Iterator<Integer> iterator = list.iterator();
        while (iterator.hasNext()) {
                int value = iterator.next();
                if (value == id) {
                    int index = list.indexOf(id);

                    if (index != -1) {
                        list.remove(index);
                    }
                }
        }
    }

Thanks in advance.

Your code seems strictly equivalent to `while (list.remove(Integer.valueOf(id)));` unless I'm missing something... — assylias, Nov 09 '12 at 11:55
@assylias. Well, you are not missing anything. You're absolutely correct. No need to iterate here at all. Nice catch :) — Rohit Jain, Nov 09 '12 at 11:56
@RohitJain Just need to make sure that `remove(Object)` is called and not `remove(int)`. — assylias, Nov 09 '12 at 11:57

score 11 · Accepted Answer · edited May 23 '17 at 12:22

11

You are removing the element in the list using the list reference itself, which can throw ConcurrentModificationException. Note that, this might work sometimes, but not always, and is not guaranteed to work perfectly.

Also, even though you use Iterator to iterate your list, you still shouldn't use list.remove, you should only use iterator.remove() to remove the elements, else it won't make any difference, whether you use iterators or enhanced for-loop.

So, use iterator.remove() to remove elements.

if (index != -1) {
    iterator.remove(value);
}

See this post: - java-efficient-equivalent-to-removing-while-iterating-a-collection for more detailed explanation.

edited May 23 '17 at 12:22

Community

1
1

answered Nov 09 '12 at 11:44

Rohit Jain

209,639
45
409
525

tbh, your answer is detailed enough :) – PermGenError Nov 09 '12 at 11:48
1

@GanGnaMStYleOverFlowErroR.. Haha :) Possibly, but that post is highly detailed to be compared with my answer. – Rohit Jain Nov 09 '12 at 11:49
Thanks, you answer is really comprehensive. – muneikh Nov 09 '12 at 12:05

score 1 · Answer 2 · answered Nov 09 '12 at 11:45

1

Simply because you're trying to remove elements from the ArrayList while iterating over them. To overcome this issue, use the java.util.concurrent.CopyOnWriteArrayList. Hope this helps.

answered Nov 09 '12 at 11:45

Egor

39,695
10
113
130

Unless the list is used in a multi-threaded environment, CopyOnWriteArrayList is not necessary. – assylias Nov 09 '12 at 11:52

Denys Séguret · Answer 3 · 2012-11-09T11:51:18.577

What happens is that the ArrayList iterator isn't designed to enable modification while you're iterating on it.

So, to avoid more serious bugs coming from incoherent data, it has a modification count which is updated when you remove an item and checked when you iterate :

From ArrayList.java :

411     public E remove(int index) {
412         rangeCheck(index);
413 
414         modCount++;
415         E oldValue = elementData(index);
416 
417         int numMoved = size - index - 1;
418         if (numMoved > 0)
419             System.arraycopy(elementData, index+1, elementData, index,
420                              numMoved);
421         elementData[--size] = null; // Let gc do its work
422 
423         return oldValue;
424     }
     ...
779 
780         final void checkForComodification() {
781             if (modCount != expectedModCount)
782                 throw new ConcurrentModificationException();
783         }

As specified in the javadoc :

The returned list iterator is fail-fast.

To avoid this problem, use the iterator to remove the current element, not the list directly. The remove method of the iterator ensures the iterator is kept coherent.

someone · Answer 4 · 2012-11-09T11:57:54.473

0

Try this

private void verifyBookingIfAvailable(ArrayList<Integer> list, int id) {

        List<Integer> tempList =new ArrayList<Integer>();
    tempList.addAll(list);

     for(Integer value :tempList) {

         if (value == 1) {
             int index = tempList.indexOf(1);

             if (index != -1) {

                 list.remove(index);
             }
         }
 }
}

while iteration you are removing objects

edited Nov 09 '12 at 11:57

answered Nov 09 '12 at 11:45

someone

6,577
7
37
60

wouldnt it still throw exception ?? – PermGenError Nov 09 '12 at 11:47
This is what you shouldn't do to modify the list to start with. Then comes iterator, which should be used. But even in iterators, you can't use `list.remove`. You have to use `iterator.remove`. So your answer doesn't help. Because, you just moved a step backward rather than moving forward. – Rohit Jain Nov 09 '12 at 11:51
I did few changes it works. though it may not be good approach – someone Nov 09 '12 at 11:59
@Sura. Yeah you are right. It works, but its not a good idea to make a copy of your list. You should straight forward use an Iterator. That's what they are there for. – Rohit Jain Nov 09 '12 at 12:06

Concurrent Modification Exception in Java

4 Answers4

Linked