Why am I getting a concurrent modification error?

Question

I have a synchronized method called getChunks(). This is the only way the set of chunks are called in the whole program, yet when iterating through them (by calling getChunks()) I get a concurrentModificationException. This would be because the ChunkManager class, which runs in a seperate thread, would have generated a new chunk. But the only way it accesses the chunks is though getChunks()...

The getChunks() method:

public synchronized Set<WorldChunk> getChunks() {
    return chunks;
}

The render() method, where the exception occurs

public void render() {
    for(WorldChunk wc : chunkMap.getChunks()) { // <-- This is the line where the exception occurs
        wc.render();
    }
}

The ChunkManager Class

public class ChunkManager implements Runnable {

    private static final int CHUNK_UPDATE_DELAY_MILLIS = 100;

    private ChunkMap chunkMap;

    public ChunkManager(ChunkMap chunkMap) {
        this.chunkMap = chunkMap;
        new Thread(this).start();
    }

    @Override
    public void run() {
        while(true) {
            manageChunks();
        }
    }

    private void manageChunks() {
        int cx = (int) ((Camera.getX() + WorldChunk.CHUNK_WIDTH / 2) / WorldChunk.CHUNK_WIDTH);
        int cz = (int) ((Camera.getZ() + WorldChunk.CHUNK_DEPTH / 2) / WorldChunk.CHUNK_DEPTH);
        int renderDistance = 2;
        for(int icx = cx - renderDistance; icx < cx + renderDistance; icx++) {
            for(int icz = cz - renderDistance; icz < cz + renderDistance; icz++) {
                if(!chunkMap.hasChunk(icx, icz)) {
                    chunkMap.genChunk(icx, icz, false);
                }
            }
        }
        for(WorldChunk wc : chunkMap.getChunks()) {
            if((Math.abs(wc.getX() - Camera.getX()) + Math.abs(wc.getZ() - Camera.getZ())) > WorldChunk.CHUNK_WIDTH + WorldChunk.CHUNK_DEPTH)  {
                wc.unload();
            }
        }
        try {
            Thread.sleep(CHUNK_UPDATE_DELAY_MILLIS);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
    }

}

EDIT StackTrace:

Exception in thread "main" java.util.ConcurrentModificationException
at java.util.HashMap$HashIterator.nextEntry(HashMap.java:894)
at java.util.HashMap$KeyIterator.next(HashMap.java:928)
at java.util.AbstractCollection.addAll(AbstractCollection.java:333)
at java.util.HashSet.<init>(HashSet.java:117)
at com.ryxuma.kalidus.world.World.render(World.java:35)
at com.ryxuma.kalidus.Core.renderPerspective(Core.java:35)
at org.heatstroke.Heatstroke$Runner.run(Heatstroke.java:365)
at org.heatstroke.Heatstroke.start(Heatstroke.java:124)
at com.ryxuma.kalidus.Core.main(Core.java:60)

Some thread updates chunks while you're in method render, you can avoid it by synchronizing around `chunkMap` — Marek Sebera, Nov 29 '13 at 17:49
@MarekSebera But I thought by synchronizing the getChunks() method, the thread that updates the chunks would wait for the iteration to finish before chaning it — Ryxuma, Nov 29 '13 at 17:52
Any stacktrace you can post? Which line throws the exception? — Daniel, Nov 29 '13 at 17:52
@Ryxuma: No; it just waits for that method to return. It doesn't have anything to do with the object it returns. — SLaks, Nov 29 '13 at 18:07
Possible duplicate of [ConcurrentModificationException despite using synchronized](http://stackoverflow.com/questions/1655362/concurrentmodificationexception-despite-using-synchronized) — Raedwald, Mar 28 '16 at 15:02

Peter Lawrey · Answer 1 · 2013-11-30T13:56:00.977

2

Your operation to get a reference to the set is thread safe, but nothing you do with that set is thread safe from that point. A simple solution is to either

return a copy of the set
lock the set while iterating
use a set which doesn't throw CME like CopyOnWriteArraySet (Which can be more expensive when you update it)

EDIT: For those interested, if you want a ConcurrentHashSet there is a trick you can use

Set <E> set = Collections.newSetFromMap(new ConcurrentHashMap<E, Boolean>());

edited Nov 30 '13 at 13:56

answered Nov 29 '13 at 17:51

Peter Lawrey

525,659
79
751
1,130

When I try to make a copy I get a error because it's changed when I iterate over it to make a new one. – Ryxuma Nov 29 '13 at 17:58
@Ryxuma This means its is being changed without being locked first. – Peter Lawrey Nov 30 '13 at 13:11
1

Thanks! Although I got it sorted by using a ConcurrentHashMap :) – Ryxuma Nov 30 '13 at 13:32
1

@Ryxuma if you want a ConcurrentHashSet there is a trick you can use `Set set = Collections.newSetFromMap(new ConcurrentHashMap());` – Peter Lawrey Nov 30 '13 at 13:46

Daniel · Accepted Answer · 2013-11-29T18:12:12.820

1

What does your wc.unload() method do?

If you try to add/remove elements from a Set while you are iterating on it, you can receive this error.

Look here: http://docs.oracle.com/javase/1.5.0/docs/api/java/util/ConcurrentModificationException.html

Specially this part:

Note that this exception does not always indicate that an object has been concurrently modified by a different thread. If a single thread issues a sequence of method invocations that violates the contract of an object, the object may throw this exception. For example, if a thread modifies a collection directly while it is iterating over the collection with a fail-fast iterator, the iterator will throw this exception.

For a solution, the easiest I can think of, keep track of all the elements you want to remove and remove them after you are done iterating by using Set.removeAll().

If you want, take a look at java.util.concurrent where you can find collections that allow you to modify as you iterate (keep an eye for how they work though, as that can be not what you expect).

edited Nov 29 '13 at 18:12

answered Nov 29 '13 at 17:55

Daniel

21,933
14
72
101

removes it from the hash map. But it is called synchronizingly(?) so it wouldn't matter? – Ryxuma Nov 29 '13 at 17:57
1

Yes, it matters. The problem is not modifying the data from different threads, but modifying the data as you iterate over it. – Daniel Nov 29 '13 at 17:59
what would you suggest then? :/ – Ryxuma Nov 29 '13 at 18:00
Added the suggestion to the answer. – Daniel Nov 29 '13 at 18:12

Why am I getting a concurrent modification error?

2 Answers2