ConcurrentModificationException even with using Collections.sychronizedMap on a LinkedHashMap [duplicate]

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)

Answer 1

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).

Answer 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.

Answer 3

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.