Fail-fast iterators throw ConcurrentModificationException

[Michael Sampson]

So this is something tricky I've seen and I know a work around but I neccesarily understand why it works. So here is my code that has been simplified quite a bit:

public class App {
    public static void main( String[] args )
    {
      Group group1 = new Group();
      Group group2 = new Group();
      group1.addRevDetail(new RevDetail("tom"));
      group2.addRevDetail(new RevDetail("dick"));
      List<Group> grouplist = Arrays.asList(new Group[]{group1,group2});
      for(Group g:grouplist){
        for(RevDetail r: g.getRevDetails()){
          g.removeRevDetail(r);
        }
      }
    }  
}

public class RevDetail {
 private String myName;
 RevDetail(String s){
   this.myName = s;
 }
}
public class Group {
  private List<RevDetail> revDetails;
  public List<RevDetail> getRevDetails(){
    return Collections.unmodifiableList(revDetails);
  }
  public void addRevDetail(RevDetail r){
    if(revDetails==null){
      revDetails = new ArrayList<RevDetail>();
    }
    revDetails.add(r);
  }
  public void removeRevDetail(RevDetail r){
    System.out.println("perform remove");
    revDetails.remove(r);
  }
}

This will cause the notorious ConcurrentModificationException. I'm paraphrasing to simplify the definition but you cannot edit a list while you are iterating over it is what that exception means. Now the work around I've found is at the g.getRevDetails() (line 10) just make that the input parameter to the constructor of a new ArrayList holding type of RevDetail and tada, the exception goes away. What I don't get is, on the getter in group for the list, it is returning a list which I am thinking is actually a copy (a whole different instance of List) that of the list that Group holds a reference to. And with the debugger it looks like I'm looking a copy of the revDetail object(s) as well. Now, when the remove is called on group, and it is passing in this copy of the revDetail object, which looks like it is able to remove somthing (even though I believe it is a copy) and then the exception is thrown. Now when I do my fix to get around this exception, I still don't really get how it is different now. Without the fix, the getter returns an unmodifieable list but when the actual remove to the physcial list is called, it is an entirely different instance of a list in memory I believe. So I don't get why that works.

[Steve Luke]

You are mistaken on two parts:

1) You say:

on the getter in group for the list, it is returning a list which I am thinking is actually a copy (a whole different instance of List)

That is not correct. The getter does Collections.unmodifiableList(). If you look at the API you will see that doesn't create a new list, it creates a view of the original list, and specifically says all queries on the List pass through to the original.

Your fix - which I think is changing

        for(RevDetail r: g.getRevDetails()){

to

        for(RevDetail r: new ArrayList<RevDetail>(g.getRevDetails())){

Does create a new list that you use to iterate over.

2) You also say:

I'm looking a copy of the revDetail object(s) as well.

That also is not correct. In the first case (where the error occurs) each r in the for loop is a new reference to the same Object in the original List. In the second case, you copy the values (references) to a new List, but this is a shallow copy - the Objects inside the List are not copied, only the reference to them. So again, each r in the for loop is a new reference to the same Object in the original List.

[Tony Docherty]

What I don't get is, on the getter in group for the list, it is returning a list which I am thinking is actually a copy (a whole different instance of List)

No. It is returning an unmodifiable view of the specified list, ie the list is wrapped in a object that allows reads through to the list but bars all modifying operations.

Now when I do my fix to get around this exception, I still don't really get how it is different now.

It works because you are iterating over a list which is a copy of the list in the Group object and when you call removeRevDetail() it calls remove on a different list to the one you are iterating over.

BTW lazy initialization of the ArrayList in the Group class should be avoided. It adds no benefit, reduces code readability and makes you class bug prone (your removeRevDetail method will throw a NPE if called before you've added any elements)

[Mike Simmons]

Tony Docherty wrote:

What I don't get is, on the getter in group for the list, it is returning a list which I am thinking is actually a copy (a whole different instance of List)

No. It is returning an unmodifiable view of the specified list, ie the list is wrapped in a object that allows reads through to the list but bars all modifying operations.

It's not actually unmodifiable. Arrays.asList() is kind of weird - it returns a List that allows some modifications and not others, depending on whether the underlying array can support that operation. So add() and remove() are not allowed, because those would need to change the length of the array, which is impossible. But set() is allowed, because that changes a value without changing the array length.

But anyway, Arrays.asList() returns a view, not a copy.

[Steve Luke]

Mike Simmons wrote:Arrays.asList() is kind of weird ...

I don't think the List he is having a problem with is the one he makes using Arrays.asList() (the List<Group>). Rather, in his getRevDetails() method he does Collections.modifiableList() to get a List<RevDetail>, and this is the list he is trying to change while iterating.

But I could be misunderstanding: I am not sure why the code needed more than one group to show the problem, or why he used Arrays.asList() to get a List to iterate - you can use the enhanced for loop on an array can't you?

[James Boswell]

This will cause the notorious ConcurrentModificationException. I'm paraphrasing to simplify the definition but you cannot edit a list while you are iterating over it is what that exception

You can edit and iterate over a list at the same time if you use an Iterator

[Mike Simmons]

Steve Luke wrote:

Mike Simmons wrote:Arrays.asList() is kind of weird ...

I don't think the List he is having a problem with is the one he makes using Arrays.asList() (the List<Group>).

Oops, you're right, I read too fast. My statement above is true fo Arrays.asList(), but irrelevant to the thread at hand. Never mind...

[Mike Simmons]

James Boswell wrote:

This will cause the notorious ConcurrentModificationException. I'm paraphrasing to simplify the definition but you cannot edit a list while you are iterating over it is what that exception

You can edit and iterate over a list at the same time if you use an Iterator

Yes - but not if the list in question is an unmodifiableList(), as in this case. That would have to change too.

[Michael Sampson]

So first off thanks for all the contributions by everyone. I didn't expect this many responses on my question but I didn't understand that 'views' even existed. Now this makes complete sense that yes I was trying to remove something from a list while I was iterating over it and thus the concurrentModificationException. So everything Steve Luke said was accurate and this was a case where I didn't understand what the debugger was doing. Not using the debugger and just putting in a bunch of print statements with object.toString() provided a lot of helpful information. The r's going into every list are new references but all refer back to the same original objects. I captured the return of the unmodifieable list in a reference and print'd the 'toString()' and got '[]' not the Class@abc123 that I expected. So I thought refernces always pointed to an object or null. I did not know what 'views' were and that you can have references to them.

[Winston Gutkowski]

Michael Sampson wrote:Now this makes complete sense that yes I was trying to remove something from a list while I was iterating over it and thus the concurrentModificationException.

Actually, even that may be incorrect. As I understand, CME is generally only thrown if your remove() is done by something outside the Iterator. Many iterator's will allow removal of its current element without throwing CME.

Winston

[Mike Simmons]

Winston Gutkowski wrote:Many iterator's will allow removal of its current element without throwing CME.

As far as I know, for all known collections that (a) support removal at all, and (b) throw CME at all, they allow removal of the current element using the iterator. It's just when you modify the Collection directly, without using the Iterator, that causes problems.

[Stephan van Hulst]

Or if you use two iterators concurrently.

[Mike Simmons]

Good point.