Search code examples
javamultithreadingsynchronizedlinkedhashmapconcurrentmodification

ConcurrentModificationException even with using Collections.sychronizedMap on a LinkedHashMap


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)

Solution

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