22

I'm occasionally getting a ConcurrentModificationException when I iterate over a list. A Google search informs me that it's probably because I'm altering that list in another thread while iterating over it and that to make this problem go away I should use java.util.concurrent.CopyOnWriteArrayList....

... except I already am.

Apparently, I'm doing something really stupid somewhere.

Does anybody have any insight into how one might induce CopyOnWriteArrayList to toss a ConcurrentModificationException? If it matters, I'm using Java 5.

Edit: Since the mutators I'm using may matter, I'm modifying this list in two ways:

  • Adding elements to the front. (list.add(0, newElement);)
  • Using subList to let older items fall off the back. (list = list.subList(0, MAX_LIST_SIZE);)

Do those raise red flags? If so, why? My understanding was that because these operations make a copy of the thing first, any existing iterators would be pointing at the unmodified original and would thus not care. Do I have a hole in my knowledge?

Edit 2: The precise code that's causing the problem is still a bit murky, but I can at least post the exception I'm seeing: 


java.util.ConcurrentModificationException
    at java.util.concurrent.CopyOnWriteArrayList$COWSubList.checkForComodification(Unknown Source)
    at java.util.concurrent.CopyOnWriteArrayList$COWSubList.iterator(Unknown Source)
    at....

... where it points to a for-each loop instantiation in my code.

That COWSubList does seem to imply that my call to subList is the root of my problem; I'd still like to understand why.

Edit 3: *facepalm*

CopyOnWriteArrayList.subList() returns a List, not a CopyOnWriteArrayList. The list it returns is under no implied obligation to provide any of COWAL's protections. Which makes using subList() like this to remove elements a Very Bad Idea.

Don't know for certain if this is my culprit, but it's damned suspicious and needs to be corrected regardless.

BlairHippo
  • 9,502
  • 10
  • 54
  • 78
  • 2
    It’s an old question, but it still should noted that `list = list.subList(0, MAX_LIST_SIZE);` is a *horrible* way to remove elements, as the sub list still references the original list which contains all elements, hence, this creates a memory leak. The canonical way of removing elements at the end, is `list.subList(MAX_LIST_SIZE, list.size()).clear();` Generally, sub lists are a tool for applying arbitrary list operations to a part of a list, so they should be held temporarily throughout the operation only. – Holger Nov 13 '18 at 07:47

2 Answers2

24

CopyOnWriteArrayList.subLists throw ConcurrentModificationExceptions if the containing list changes out from underneath it:

public class ListTest {

  private static List<int[]> intList;

  public static void main (String[] args) {
    CopyOnWriteArrayList<Integer> cowal = new CopyOnWriteArrayList<Integer>();
    cowal.add(1);
    cowal.add(2);
    cowal.add(3);

    List<Integer> sub = cowal.subList(1, 2);
    cowal.add(4);
    sub.get(0); //throws ConcurrentModificationException
  }
}
Sbodd
  • 11,279
  • 6
  • 41
  • 42
  • A am indeed making a call to subList, so this could be it. (Hard to tell, as the error is occurring in the field, not on my test rig. Grr.) However, to make that more like my code, I changed "cowal" to a simple "List", and didn't bother with the intermediate "sub" variable; I did "cowal = cowal.subList(1, 2);". Under those conditions, the code runs without incident. Is there any other way for a subList() call to cause trouble? – BlairHippo Oct 06 '09 at 19:38
  • matt b: I would, but too much code and I haven't figured out how to strip it down to where it throws the error. (Yet.) If I come up with a bare-bones version (and the answer DOESN'T jump up and smack me in the face), I'll post it. – BlairHippo Oct 06 '09 at 20:16
  • Still haven't precisely replicated the error locally, but I'm 90% sure that subList() is indeed the source of my woe. (See the edited question for details why.) My thanks for getting me pointed in the right direction. – BlairHippo Oct 07 '09 at 14:40
-1

Sbodd has the correct answer, but it sounds like using CopyOnWriteArrayList instead of ArrayList is just an attempt to mask the error. The true problem is an attempt to modify the underlying list while iterating over it. You need to find where in your code you are accessing it as such and remove that usage or work around it.

matt b
  • 138,234
  • 66
  • 282
  • 345
  • 8
    Except, isn't safely modifying the underlying list while iterating over it CopyOnWriteArrayList's reason for existing? – BlairHippo Oct 06 '09 at 20:19