Java - concurrent modification exception

I am getting a concurrent modification exception in the following code:

        for(Iterator<Tile> iter = spawner.activeTiles.iterator(); iter.hasNext();) {
            Tile tile = iter.next();
            canvas.drawRect(tile, tile.getColor());
        }

      

I understand that concurrent modification happens when it changes during iteration (add / remove inside iteration). I also understand that they can happen when multithreading is where I think my problem is.

In my game, I have several timers that run on threads. I have a developer who adds values ​​to activeTiles on every tick. Then I have 2 timers, one for fading out and one for fading out. Without discarding my game, the tile is basically removed from the list when the fade ends, or when the player removes the tile. Thus, there are several cases where fragments are removed from the list of fragments:

for(Iterator<Tile> iter = spawner.activeTiles.iterator(); iter.hasNext();) {
                Tile tile = iter.next();

                if(tile.contains(x, y) && tile.equals(spawner.activeTiles.get(0))) {
                    vibrator.vibrate(50);

                    tile.setBroken(true);
                    score ++;
                    spawner.setTileDelayInfo();
                    iter.remove();

      

and before each new appearance, it removes all the failed fragments:

private void removeFailedTiles() {
    for(Iterator<Tile> iter = activeTiles.iterator(); iter.hasNext();) {
        Tile tile = iter.next();

        if(tile.isFailed()) {
            iter.remove();
        }
    }
}

      

It seems like it happens by accident. So I think it should do something over time, but I'm new to this kind of exception and don't know what to look for or why this is happening.

+3


source to share


4 answers


Multithreading can be the source of ConcurrentModificationException

s. This can happen when one thread is modifying the structure of a collection and another thread is Iterator

iterating over it. This can lead to unexpected states in your application when the state of the collection changes, when a section of code requires a consistent representation of the data. This is required when you are iterating over assembly Tile

s.

You need to synchronize access to the collection activeTiles

. Anything that modifies this collection structurally (adds or removes) or iterates over it must synchronize in this collection.

Add a block synchronized (activeTiles)

around all of the code that is iterated over or structurally modified activeTiles

. This includes all 3 code snippets that you provided here.



Alternatively, you can do 3 methods that match your code snippets synchronized

.

In any case, no one else Thread

can execute any of the blocks synchronized

until the other Thread

finishes its section syncrhonized

, preventing ConcurrentModificationException

.

+1


source


The good news: you have eradicated the root cause of the problem in your question - you cannot have multiple threads accessing a list at the same time, unless they are just reading.

You can solve this problem in one of two ways, depending on how the rest of your code works. The most "correct" way is steffen's answer: any access to the list must be block-protected synchronized

, and this includes keeping a lock for the full duration of all iterations of the list. Note, if you do this, you want to do as little work as possible while holding the lock - in particular, it is a bad idea to make any listener calls while holding the lock.



The second option is to use CopyOnWriteArrayList , which is thread safe and does not require any external synchronization - but any changes to the list (add / remove / replace calls) are significantly more expensive.

+2


source


It is unsafe to delete items from Iterator

that support deletion of items when you are re-assembling on a different thread.

Get Lock on all threads on activeTiles

before repeating them.

+1


source


You might want to make your list thread safe. Use Collections.synchronizedList()

.

threadSafeActiveTiles = Collections.synchronizedList(activeTiles);

      

Remember, you must keep this list in sync when you iterate over it:

synchronized (threadSafeActiveTiles) {
    for (Iterator<Tile> it = threadSafeActiveTiles.iterator(); it.hasNext();) {
        Tile tile = it.next();
        // ...
    }
}

      

Then you can safely have multiple threads modifying the list, which seems like your business.

List returned Collections.synchronizedList()

, eliminates the need to use a synchronized block (above) in separate operations in this list, for example add(e)

, size()

, get(i)

and etc ....

+1


source







All Articles