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)

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 — 王奕然
– 王奕然, Commented Aug 28, 2013 at 8: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
– mpellegr, Commented Aug 28, 2013 at 18:13

yshavit · Accepted Answer · 2013-08-20 20:16:53Z

13

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, 2013 at 20:16

answered Aug 20, 2013 at 20:09

yshavit

43.7k8 gold badges90 silver badges127 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

mpellegr Over a year ago

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?

yshavit Over a year ago

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.

mpellegr Over a year ago

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?

yshavit Over a year ago

There's not really a good way, unfortunately. This sort of complexity is one of the reasons immutable objects are popular.

mpellegr Over a year ago

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!

Antoniossss · Accepted Answer · 2013-08-20 20:09:46Z

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, 2013 at 20:09

Antoniossss

32.7k7 gold badges67 silver badges119 bronze badges

Comments

Costi Ciudatu · Accepted Answer · 2013-08-20 20:23:01Z

2

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.

edited Aug 20, 2013 at 20:23

answered Aug 20, 2013 at 20:09

Costi Ciudatu

38.5k7 gold badges58 silver badges95 bronze badges

3 Comments

mpellegr Over a year ago

I place all instances of iterating on the map in a synchronization block.

Costi Ciudatu Over a year ago

I see... I totally missed that; sorry ! I'll edit my answer to give you a hint.

Joonas Vali Over a year ago

Tha internal mutex actually is 'this' on default cases, provided in constructor.

Collectives™ on Stack Overflow

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

3 Answers 3

5 Comments

Comments

3 Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

5 Comments

Comments

3 Comments

Linked

Related