17

The following Java code throws a ConcurrentModificationException, as expected:

public class Evil
{
    public static void main(String[] args) {
        Collection<String> c = new ArrayList<String>();
        c.add("lalala");
        c.add("sososo");
        c.add("ahaaha");
        removeLalala(c);
        System.err.println(c);
    }
    private static void removeLalala(Collection<String> c) 
    {
        for (Iterator<String> i = c.iterator(); i.hasNext();) {
            String s = i.next();
            if(s.equals("lalala")) {
                c.remove(s);
            }
        }
    }
}

But the following example, which differs only in the contents of the Collection, executes without any exception:

public class Evil {
    public static void main(String[] args) 
    {
        Collection<String> c = new ArrayList<String>();
        c.add("lalala");
        c.add("lalala");
        removeLalala(c);
        System.err.println(c);
    }
    private static void removeLalala(Collection<String> c) {
        for (Iterator<String> i = c.iterator(); i.hasNext();) {
            String s = i.next();
            if(s.equals("lalala")) {
                c.remove(s);
            }
        }
    }
}

This prints the output "[lalala]". Why doesn't the second example throw a ConcurrentModificationException when the first example does?

Kevin J. Chase
  • 3,856
  • 4
  • 21
  • 43
Ravi Kuldeep
  • 233
  • 1
  • 10

5 Answers5

22

Short answer

Because the fail-fast behavior of an iterator isn't guaranteed.

Long answer

You're getting this exception because you cannot manipulate a collection while iterating over it, except through the iterator.

Bad:

// we're using iterator
for (Iterator<String> i = c.iterator(); i.hasNext();) {  
    // here, the collection will check it hasn't been modified (in effort to fail fast)
    String s = i.next();
    if(s.equals("lalala")) {
        // s is removed from the collection and the collection will take note it was modified
        c.remove(s);
    }
}

Good:

// we're using iterator
for (Iterator<String> i = c.iterator(); i.hasNext();) {  
    // here, the collection will check it hasn't been modified (in effort to fail fast)
    String s = i.next();
    if(s.equals("lalala")) {
        // s is removed from the collection through iterator, so the iterator knows the collection changed and can resume the iteration
        i.remove();
    }
}

Now to the "why": In the code above, notice how the modification check is performed - the removal marks the collection as modified, and next iteration checks for any modifications and fails if it detects the collection changed. Another important thing is that ArrayList (not sure about other collections) does not check for modification in hasNext().

Therefore, two strange things may happen:

  • If you remove the last element while iterating, nothing will be thrown
    • That's because there's no "next" element, so the iteration ends before reaching the modification-checking code
  • If you remove the second-to-last element, ArrayList.hasNext() will actually also return false, because the iterator's current index is now pointing at the last element (former second-to-last).
    • So even in this case, there's no "next" element after the removal

Note that this all is in line with ArrayList's documentation:

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.

Edited to add:

This question provides some information on why the concurrent modification check is not performed in hasNext() and is only performed in next().

Community
  • 1
  • 1
Jiri Tousek
  • 12,211
  • 5
  • 29
  • 43
  • 1
    in both the case remove function is same. My question is why it is not giving error in the second case but in first case it is giving error? – Ravi Kuldeep Nov 25 '15 at 10:26
  • 1
    Actually, it doesn't matter which element you remove, it only matters what the current element is, i.e. where the iterator is positioned. Any position other than second-last will throw exception, but if iterator is on second-last element and you remove *any* element, the iteration stops without error, and without iterating the last element. – Andreas Nov 25 '15 at 10:41
  • but the moment the collection is altered shouldn't it give an exception at that moment?? I don't know the code behind hasnext() ,next() and remove(Object o). Can you please explain me in reference to implementation of these codes. – Ravi Kuldeep Nov 25 '15 at 10:47
  • @Andreas you're right. The usual error is to try to remove the "current" element through the collection instead of through iterator however, so I'll stick to that. – Jiri Tousek Nov 25 '15 at 10:48
  • 1
    @RaviKuldeep it is not possible to raise the exception when the collection is modified, because the collection does not know about any existing iterators (it cannot know what iterators are still "alive" even if it tracked the created iterators), so the only possible point to check for modification is when the iterator is accessed. It could be possible to check in `Iterator.hasNext()`, but Java doesn't do that for some reason. – Jiri Tousek Nov 25 '15 at 11:22
  • @JiriTousek Thanks got it!!! But one last doubt if the element is removed through iterator wouldn't it be treated as some modification in the collection. So why removing it through iterator makes it a safe way to do it? – Ravi Kuldeep Nov 25 '15 at 11:43
  • @RaviKuldeep theoretically, because the iterator knows that a change happened and what the change was, so it can adjust accordingly (e.g. lower its internal `current index` so that the next element will be chosen correctly). Technically, the modification detection is based on `modificationCount` internal field that the iterator copies on creation and checks for equality later. As a part of removal through the iterator, I expect the iterator will update it's internal copy of `modificationCount` to match the new value in the collection. – Jiri Tousek Nov 25 '15 at 11:47
4

If you look at the source code for the ArrayList iterator (private nested class Itr), you'll see the flaw in the code.

The code is supposed to be fail-fast, which is done internally in the iterator by calling checkForComodification(), however the hasNext() doesn't make that call, likely for performance reasons.

The hasNext() instead is just:

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

This means that when you're on the second last element of the list, and then remove an element (any element), the size is reduced and hasNext() thinks you're on the last element (which you weren't), and returns false, skipping the iteration of the last element without error.

OOPS!!!!

Andreas
  • 154,647
  • 11
  • 152
  • 247
  • I checked the implementations of hasnext(), next() and remove(Object o) but couldn't understand them perfectly. Can you please explain the answer in regard to those implementations? – Ravi Kuldeep Nov 25 '15 at 10:52
  • @RaviKuldeep I showed the implementation of `hasNext()` and explained how/why it's failing. I'm not about to explain in detail how the entire iterator class works. If you can't understand the code, just accept the fact that it works when used correctly, and doesn't guarantee failure when used incorrectly. Later, when you understand Java better, you can look at it again. – Andreas Nov 25 '15 at 10:56
3

From other answers you know what is the right way of removing an element in collection while you are iterating the collection. I give here the explanation to the basic question.And the answer to your question lies in the below stack trace

Exception in thread "main" java.util.ConcurrentModificationException
    at java.util.ArrayList$Itr.checkForComodification(Unknown Source)
    at java.util.ArrayList$Itr.next(Unknown Source)
    at com.ii4sm.controller.Evil.removeLalala(Evil.java:23)
    at com.ii4sm.controller.Evil.main(Evil.java:17)

In the stacktrace it is obvious that i.next(); line throws the error. But when you have only two elements in the collection.

Collection<String> c = new ArrayList<String>();
c.add("lalala");
c.add("lalala");
removeLalala(c);
System.err.println(c);

When the first one is removed i.hasNext() returns false and i.next() is never executed to throw the exception

awsome
  • 2,143
  • 2
  • 23
  • 41
2

you should remove from the iterator (i) not the collection (c) directly;

try this:

for (Iterator<String> i = c.iterator(); i.hasNext();) {
    String s = i.next();
    if(s.equals("lalala")) {
        i.remove(); //note here, changing c to  i with no parameter.
    }
}

EDIT:

The reason why the first try throws exception while the second one doesn't is simply because of the number of element in your collection.

as the first one will go through the loop more than once and the second one will only iterate once. therefore, it doesn't have the chance to throw exception

nafas
  • 5,283
  • 3
  • 29
  • 57
  • in both the case remove function is same. My question is why it is not giving error in the second case but in first case it is giving error? – Ravi Kuldeep Nov 25 '15 at 10:24
  • @RaviKuldeep I've updated my answer – nafas Nov 25 '15 at 10:29
  • but the moment the collection is altered shouldn't it give an exception at that moment?? I don't know the code behind hasnext() ,next() and remove(Object o). Can you please expain me in referernce to implementation of these codes.Correct me if I am understanding it wrong – Ravi Kuldeep Nov 25 '15 at 10:35
  • @RaviKuldeep no, this problem occurs because of the loop, try the second approach with 3 elements and u should get the same error – nafas Nov 25 '15 at 10:37
1

You can't remove from list if you're browsing it with "for each" loop.

You can't remove an item from a collection you're iterating over. You can get around this by explicitly using an Iterator and removing the item there. You can use Iterator.

If you use below code you will not get any exception :

private static void removeLalala(Collection<String> c) 
  {
    /*for (Iterator<String> i = c.iterator(); i.hasNext();) {
      String s = i.next();
      if(s.equals("lalala")) {
        c.remove(s);
      }
    }*/

    Iterator<String> it = c.iterator();
    while (it.hasNext()) {
        String st = it.next();
        if (st.equals("lalala")) {
            it.remove();
        }
    }
  }
Shiladittya Chakraborty
  • 4,270
  • 8
  • 45
  • 94