ConcurrentModificationException even with using Collections.sychronizedMap on a LinkedHashMap

Question

I'm using a Map object in my class that I've synchronized with Collections.synchronizedMap() for a LinkedHashMap like so:

private GameObjectManager(){
        gameObjects = Collections.synchronizedMap(new LinkedHashMap<String, GameObject>());
}

I'm getting a concurrent modification exception on the third line of this function:

public static void frameElapsed(float msElapsed){
    if(!INSTANCE.gameObjects.isEmpty()){
        synchronized(INSTANCE.gameObjects){
            for(GameObject object : INSTANCE.gameObjects.values()){...}
        }
    }
}

All other locations where I am iterating through the Map, I am synchronizing on the map per the docs.

There are other functions in my class that use this Map (the synchronized one!) and they put() and remove() objects, but this shouldn't matter though. What am I doing wrong? Please ask for more code, not sure what else to put.

Oh, and the log message:

08-20 15:55:30.109: E/AndroidRuntime(14482): FATAL EXCEPTION: GLThread 1748
08-20 15:55:30.109: E/AndroidRuntime(14482): java.util.ConcurrentModificationException
08-20 15:55:30.109: E/AndroidRuntime(14482):    at     java.util.LinkedHashMap$LinkedHashIterator.nextEntry(LinkedHashMap.java:350)
08-20 15:55:30.109: E/AndroidRuntime(14482):    at     java.util.LinkedHashMap$ValueIterator.next(LinkedHashMap.java:374)
08-20 15:55:30.109: E/AndroidRuntime(14482):    at     package.GameObjectManager.frameElapsed(GameObjectManager.java:247)
08-20 15:55:30.109: E/AndroidRuntime(14482):    at     package.GamekitInterface.render(Native Method)
08-20 15:55:30.109: E/AndroidRuntime(14482):    at     package.GamekitInterface.renderFrame(GamekitInterface.java:332)
08-20 15:55:30.109: E/AndroidRuntime(14482):    at     com.qualcomm.QCARSamples.ImageTargets.GameEngineInterface.onDrawFrame(GameEngineInterface.java:107)
08-20 15:55:30.109: E/AndroidRuntime(14482):    at     android.opengl.GLSurfaceView$GLThread.guardedRun(GLSurfaceView.java:1516)
08-20 15:55:30.109: E/AndroidRuntime(14482):    at     android.opengl.GLSurfaceView$GLThread.run(GLSurfaceView.java:1240)

you should use **gameObjects** whatever functions use if you call twice GameObjectManager();the first gameObjects and the second gameObjects are not the same object,so may cause ConcurrentModificationException — 王奕然, Aug 28 '13 at 08:42
I don't understand what you said. But I did notice that I should do that isEmpty test after I synchronize. Is that what you said? — mpellegr, Aug 28 '13 at 18:13

yshavit · Accepted Answer · 2013-08-20T20:16:53.393

12

Despite the name, this has nothing to do with concurrency in the multithreading sense. You can't modify this map while iterating on it, except by invoking remove() on the iterator. That is, where you have...

for(GameObject object : INSTANCE.gameObjects.values()){...}

if the ... modifies INSTANCE.gameObjects.values() (for instance, removing or adding an element), the next invocation to next() on the iterator (which is implicit to the for loop) will throw that exception.

This is true of most collections and Map implementations. The javadocs usually specify that behavior, though not always obviously.

Fixes:

If what you're trying to do is remove the element, you need to explicitly get the Iterator<GameObject> and call remove() on it.

for (Iterator<GameObject> iter = INSTANCE.getObjects().values(); iter.hasNext(); ;) {
     GameObject object = iter.next();
     if (someCondition(object)) {
         iter.remove();
     }
 }

If you're trying to add an element, you need to create a temporary collection to hold the elements you want to add, and then after the iterator is finished, putAll(temporaryMapForAdding).

edited Aug 20 '13 at 20:16

answered Aug 20 '13 at 20:09

yshavit

42,327
7
87
124

I'm not adding or removing anything in any of the iterations I do. I am adding and removing in other functions that are called from another thread. Is this not allowed? – mpellegr Aug 20 '13 at 20:15
1

Well yes, that's the whole point of the `synchronized` block. The other thread won't be able to invoke any methods on the map at all until the `synchronized` block is finished. But that's not causing this problem; this problem would exist with just one thread. – yshavit Aug 20 '13 at 20:18
Ah, I see what's happening! Through a long trace of function calls, objects are getting added to the map during the iteration. I'll mark this as the answer, but do you know of a way I can use the logic already in place that is adding/removing objects from the map without having to worry if I'm iterating or not? – mpellegr Aug 20 '13 at 20:25
There's not really a good way, unfortunately. This sort of complexity is one of the reasons immutable objects are popular. – yshavit Aug 20 '13 at 20:31
That's a bummer. I might be able to make a copy of the map and iterate over potentially stale values, but at least I won't have to refactor a ton of code. Thanks for your help! – mpellegr Aug 20 '13 at 20:35

Costi Ciudatu · Answer 2 · 2013-08-20T20:23:01.547

Collections.synchronizedMap() doesn't help you when iterating. That will just make your map perform the put/get/remove operations atomically (that means you won't have two such operations running simultaneously).

When iterating, you fetch each element and do something with it. But what if the very element that you are acting upon within your iteration as your current element gets removed by some other thread ?

This is what the exception is trying to prevent, as you may get a result that doesn't correspond to any actual snapshot of your map: if you're computing the sum of your Integer values, for example, the elements you already added may get removed and others may be added while you iterate, so you'll end up with a sum that doesn't match any "snapshot" of your map.

For what you're trying to do, the only solution would be to perform the whole iteration within some synchronized block but it's mandatory that you synchronize on the same monitor used by your map operations. And the Collections.syncrhonizedMap() provides a wrapper which synchronizes on some internal mutex, not on the this reference. Therefore, your attempt to prevent any modification to your map while iterating will fail.

I place all instances of iterating on the map in a synchronization block. — mpellegr, Aug 20 '13 at 20:18
I see... I totally missed that; sorry ! I'll edit my answer to give you a hint. — Costi Ciudatu, Aug 20 '13 at 20:21
Tha internal mutex actually is 'this' on default cases, provided in constructor. — Joonas Vali, Jun 28 '16 at 09:52

score 2 · Answer 3 · answered Aug 20 '13 at 20:09

2

you are using for-each alike version of for loop. In Java it is forbidden to add or remove elements from iterated collection in such loop. To avoid this situation, use collections iterator. From iterator, you can remove elements.

answered Aug 20 '13 at 20:09

Antoniossss

31,590
6
57
99

ConcurrentModificationException even with using Collections.sychronizedMap on a LinkedHashMap

3 Answers3

Linked