ConcurrentModificationException when updating sprite images

Question

I keep getting a ConcurrentModificationException when running my game which utilizes multithreading to create new sprites and move them. The main problem appears to happen with the creation and/or movement of "Fireballs".

I've been able to run my program successfully with no exceptions appearing by commenting out the createNewFireballs method. However, whenever I do utilize the createNewFireballs method, the error commonly appears whenever I call a function that updates the image of the Fireball Sprite (and doesn't ever happen for any other type of sprite.) I was wondering if anyone could help me locate the source of my problem and potentially solve the issue.

public synchronized void createNewFireball() {
    new Thread(new Runnable() {
        public void run() {
            while (gameNotPaused && hero.isNotDead()) {
                fireball = new Fireball(dragon.getX(), dragon.getY());
                fireballs.add(fireball);
            try
                {
                Thread.sleep(100); //Waits for .1 second
                }
            catch(InterruptedException e)
                {
                e.printStackTrace();
                Thread.currentThread().interrupt();
                }
            }
        }
    }).start();
}

    //The problem commonly occurs in the update method, 
    //specifically the line  "FireballIter.next().updateImage(g);"
public synchronized void update(Graphics g) {
    Iterator<Sprite> FireballIter = fireballs.iterator();

    Iterator<Sprite> arrowIter = arrows.iterator();
    while (arrowIter.hasNext()) {
        arrowIter.next().updateImage(g);
    }

    Iterator<Sprite> iterator = sprites.iterator(); 
    while (iterator.hasNext()) {
        iterator.next().updateImage(g);
    }

    while (FireballIter.hasNext()) {
        FireballIter.next().updateImage(g);
    }

}
//Although sometimes it occurs as a result of updateScene, which is 
//called in another method which moves all the "projectile" sprites
public synchronized void updateScene(int width, int height) {

    Iterator<Sprite> arrowIter = arrows.iterator();
    while(arrowIter.hasNext()) {
        Sprite spriteObject = arrowIter.next(); 
        ((Arrow) spriteObject).updateState();   
        if (spriteObject.overlaps(dragon, 350, 350)) {
            dragon.arrowHit();
            System.out.printf("Dragon was hit at %d, %d%n, while arrow was at %d,%d%n", dragon.getX(), dragon.getY(), spriteObject.getX(), spriteObject.getY());
            arrowIter.remove();
        }
    }

    Iterator<Sprite> fireballIter = fireballs.iterator();
    while(fireballIter.hasNext()) {
        Sprite spriteObject = fireballIter.next(); 
        ((Fireball) spriteObject).updateState();    
    }
}

@Override
public synchronized void run() {
    while (model.getGameNotPaused()) {
        model.updateScene(view.getWidth(), view.getHeight());
        view.repaint();
        try {
            Thread.sleep(2);
        } catch (InterruptedException e) {
            e.printStackTrace();
            Thread.currentThread().interrupt();
            JOptionPane.showMessageDialog(null, "Press R to resume game.");
        }
    }
}

Answer 1

Making createNewFireball synchronized does nothing useful: the synchronization only applies to the execution of that method, not the runnable executed by the thread (which is good, because otherwise none of the other methods would be able to execute until the while loop finished).

Put the fireballs.add into a synchronized block, taking care to ensure you are synchronizing on the right thing:

• If you simply use synchronized (this), you would be synchronizing on the Runnable.
• Instead, use synchronized (TheContainingClass.this) (where TheContainingClass is the name of the class containing these methods).