I can't find the cause of my java.util.ConcurrentModificationException

Question

I have code that enters the for loop within my Main method.

for (List<Point2D> points : output) {
    currentPath = pathDistance(points);
    if (shortest == 0){
        shortest = currentPath;
    } else if (currentPath < shortest) {
        best = points;
        shortest = currentPath;
    }
}

where pathDistance is defined as

public static Double pathDistance(List<Point2D> path){
    double distance = 0;
    int count = path.size()-1;

    for (int i = 0; i < count; i++) {
        distance = distance + path.get(i).distance(path.get(i+1));
    }

    distance = distance + path.get(0).distance(path.get(count));
    return distance;
}

But I keep getting the error

Exception in thread "main" java.util.ConcurrentModificationException
   at java.util.SubList.checkForComodification(Unknown Source)
   at java.util.SubList.size(Unknown Source)
   at java.util.Collections$SynchronizedCollection.size(Unknown Source)
   at TSMain.pathDistance(TSMain.java:76)
   at TSMain.main(TSMain.java:203)

I know this is supposed to mean that I am altering an object while an iteration depends on it, but I can't for the life of me figure out where that might be happening. Any help would be appreciated.

You're altering `output`, but not necessarily in this `for` loop. This can also be done by another thread in another piece of code, but with the exact same reference to the `output` object. — Tom, Sep 14 '16 at 19:25
Try to replace the enhanced for loop to the traditional method where it calculates the size at each iteration. If you modify the list at all (remove any elements or add any elements), you will run into issues in the enhanced for loop. — A.Sharma, Sep 14 '16 at 19:30
It looks like you are passing sublist wrapped by `Collections.synchronizedCollection`. However original list is getting modified while you iterating over it. — vsminkov, Sep 14 '16 at 19:33

vsminkov · Answer 1 · 2016-09-14T22:06:23.870

Your stacktrace shows that somewhere in your code subList is passed to Collections.synchronizedCollection (directly or indirectly). Like this

Set<List<Point2D>> output = Collections.singleton(
    Collections.synchronizedCollection(data.subList(start, end)));

However it does not copy data list. And points subList is still pointing to a range in data list. And original list is modified at the momet path.size() call occurs.

You can easily fix your problem by doing explicit list copy before passing it to pathDistance

for(List<Point2D> points : output){
    List<Point2D> pointsCopy = new ArrayList<>(points)
    currentPath = pathDistance(pointsCopy);
    // rest of code using pointsCopy
}

I also should notice that it looks like there is a problem with synchronization in your code. Wrapping sublists in synchronized collection is a bad idea because original list could be modified in unsafe manner w/o proper synchronization.

You can learn more about list modification checking by looking into AbstractList#modCount sources.

I can't find the cause of my java.util.ConcurrentModificationException

1 Answers1