How to avoid the ConcurrentModificationException

Question

I am trying to remove items from an array list with an iterator and I keep getting the ConcurrentModificationExceptionhere is my code:

public void forward() 
{
    for (Sprite s : sprites)
    {
        s.move();
        for (Iterator<Sprite> iter = sprites.iterator(); iter.hasNext();) 
        {  
            s = iter.next();
            if (s instanceof Attacker)
            {
                for (Sprite s2  : sprites)
                {
                    if(s.overlaps(s2))
                        s2.hit();
                }
            }
            if (s.shouldRemove())
                iter.remove();
        }
    }
}

it works for about the first 15 to 20 times and then I get the error every couple clicks

Exception in thread "AWT-EventQueue-0" java.util.ConcurrentModificationException
at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:901)
at java.util.ArrayList$Itr.next(ArrayList.java:851)
at Model.forward(Model.java:46)
at Controller.mousePressed(Controller.java:29)
at java.awt.Component.processMouseEvent(Component.java:6522)
at javax.swing.JComponent.processMouseEvent(JComponent.java:3321)
at java.awt.Component.processEvent(Component.java:6290)
at java.awt.Container.processEvent(Container.java:2234)
at java.awt.Component.dispatchEventImpl(Component.java:4881)
at java.awt.Container.dispatchEventImpl(Container.java:2292)
at java.awt.Component.dispatchEvent(Component.java:4703)
at java.awt.LightweightDispatcher.retargetMouseEvent(Container.java:4898)
at java.awt.LightweightDispatcher.processMouseEvent(Container.java:4530)
at java.awt.LightweightDispatcher.dispatchEvent(Container.java:4462)
at java.awt.Container.dispatchEventImpl(Container.java:2278)
at java.awt.Window.dispatchEventImpl(Window.java:2739)
at java.awt.Component.dispatchEvent(Component.java:4703)
at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:746)
at java.awt.EventQueue.access$400(EventQueue.java:97)
at java.awt.EventQueue$3.run(EventQueue.java:697)
at java.awt.EventQueue$3.run(EventQueue.java:691)
at java.security.AccessController.doPrivileged(Native Method)
at java.security.ProtectionDomain$1.doIntersectionPrivilege(ProtectionDomain.java:75)
at java.security.ProtectionDomain$1.doIntersectionPrivilege(ProtectionDomain.java:86)
at java.awt.EventQueue$4.run(EventQueue.java:719)
at java.awt.EventQueue$4.run(EventQueue.java:717)
at java.security.AccessController.doPrivileged(Native Method)
at java.security.ProtectionDomain$1.doIntersectionPrivilege(ProtectionDomain.java:75)
at java.awt.EventQueue.dispatchEvent(EventQueue.java:716)
at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:201)
at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:116)
at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:105)
at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:93)
at java.awt.EventDispatchThread.run(EventDispatchThread.java:82)

I am not entirely sure which one throws the errors

which line in your example is line 46 in your file ("Model.forward(Model.java:_46_)")? — jtahlborn, Oct 17 '14 at 19:48
the hit function changes it to a different picture and stops it from moving — cwendel, Oct 17 '14 at 20:08
Possible duplicate of [Iterating through a Collection, avoiding ConcurrentModificationException when removing in loop](http://stackoverflow.com/questions/223918/iterating-through-a-collection-avoiding-concurrentmodificationexception-when-re) — Raedwald, Mar 28 '16 at 14:40

John Bollinger · Accepted Answer · 2014-10-17T20:25:22.063

3

Your get a ConcurrentModificationException because you remove an element from a collection while you are iterating over that collection, other than via the iterator. In this case, you have no explicit iterator for the outer iteration, so there is no way to safely modify the sprites collection inside the loop.

The best you can do is probably to collect the elements to delete into a temporary collection, then remove them all after the loop, something like this:

Set<Sprite> toRemove = new HashSet<Sprite>();

for (Sprite s1 : sprites) {
    if (toRemove.contains(s1)) {
        continue;
    }
    s1.move();
    for (Sprite s : sprites) {
        if (toRemove.contains(s)) {
            continue;
        }
        if (s instanceof Attacker) {
            for (Sprite s2  : sprites) {
                if (toRemove.contains(s2)) {
                    continue;
                }
                if(s.overlaps(s2)) {
                    s2.hit();
                }
            }
        }
        if (s.shouldRemove()) {
            toRemove.add(s);
        }
    }
}

sprites.removeAll(toRemove);

edited Oct 17 '14 at 20:25

answered Oct 17 '14 at 19:52

John Bollinger

160,171
8
81
157

holy cow, how did i miss that! i read that code 10 times and missed that outer loop. – jtahlborn Oct 17 '14 at 19:58
1

Delaying removal could change behavior if any of the methods invoked on `Sprite` instances (`move()`, `overlaps(Sprite)`, `hit()`, and `shouldRemove()`) use the `sprites` collection. If that's the case, then the whole thing probably needs to be reconsidered. – John Bollinger Oct 17 '14 at 20:06
heh, yeah, there are some problems in the code logic besides the CME. for instance, hits could hit things earlier in the list which may not get removed in a timely manner. – jtahlborn Oct 17 '14 at 20:08
I'm not really sure what the `Set` and `HashSet` do you have any idea how to do it without this? I appreciate the answer and I understand the problem now, but the answer was a little out of my league :/ The idea is to remove them after a consecutive number of mouse clicks after they were initially hit. I need to remove them 1 by 1 – cwendel Oct 17 '14 at 20:10
`Set` is an interface representing a different type of collection. `HashSet` is an appropriate implementation of that interface. You could instead substitute `List` and `ArrayList`, since it appears you are more familiar with those. – John Bollinger Oct 17 '14 at 20:17
That did it. Seriously appreciate it. That was extremely frustrating – cwendel Oct 17 '14 at 20:23
Anyway, provided that `Sprite` instances don't look at or operate on the overall collection of sprites (as mentioned earlier), the revised code I offered will perform exactly the same operations on exactly the same sprites as your code seems to indicate you wanted to do, and it will leave the sprite collection in the state you appear to have intended. – John Bollinger Oct 17 '14 at 20:23

How to avoid the ConcurrentModificationException

1 Answers1