0
package biz.boulter.state;

import java.awt.Color;
import java.awt.Graphics2D;
import java.util.ArrayList;

import biz.boulter.sword.Game;
import biz.boulter.sword.Particle;

public class Menu implements State{
private ArrayList<Particle> particles = new ArrayList<Particle>();
boolean inLoop = false;

public Menu(){

}

@Override
public void render(Graphics2D g) {
    if(!inLoop){
        for(Particle p: particles){
            p.render(g);
        }
    }
}

@Override
public void tick() {
    if(!inLoop){
        for(Particle p: particles){
            p.tick();
        }
    }
}

@Override
public void keyPressed(int kc) {

}

@Override
public void keyReleased(int kc) {

}

@Override
public void mousePressed(int button, int x, int y) {
    for(int i = 0; i<500; i++){
        int rand;
        if(Game.rand.nextBoolean()){
            rand = Game.rand.nextInt(10)-11;
        }else{
            rand = Game.rand.nextInt(10)+1;
        }
        particles.add(new Particle(x, y, rand, Game.rand.nextInt(10)-11, new Color(Game.rand.nextInt(1000000000))));
    }
}

@Override
public void mouseReleased(int button, int x, int y) {

}
}

Hey guys this is my code and i keep getting ConcurrentModificationException. And i know this means that two things are changing the particles variable at the same time. But how else am i supposed to add to the array list. I saw some forums that said use Iterator but that is for removing not adding.

Thanks in advance!

EDIT:

stacktrace:

Exception in thread "Thread-2" java.util.ConcurrentModificationException
    at java.util.ArrayList$Itr.checkForComodification(Unknown Source)
    at java.util.ArrayList$Itr.next(Unknown Source)
    at biz.boulter.state.Menu.render(Menu.java:21)
    at biz.boulter.sword.Game.render(Game.java:43)
    at biz.boulter.sword.Game.run(Game.java:136)
at java.lang.Thread.run(Unknown Source)

particle class:

package biz.boulter.sword;

import java.awt.Color;
import java.awt.Graphics2D;
import java.awt.Rectangle;

public class Particle {
private double x = 0;
private double y = 0;
private double xa = 0;
private double ya = 0;
private Color particleColour;
private Rectangle img;

public Particle(int x, int y, int xa, int ya, Color colour){
    this.x = x;
    this.y = y;
    this.xa = xa;
    this.ya = ya;
    this.particleColour = colour;
    img = new Rectangle(x, y, 5, 5);
}

public void render(Graphics2D g){
    img.setBounds((int)Math.round(x), (int)Math.round(y), 5, 5);
    g.setColor(particleColour);
    g.fill(img);
}

public void tick(){
    ya+=0.5;

    if(xa < 0){
        xa+=1;
    }

    if(xa > 0){
        xa-=1;
    }

    x+=xa;
    y+=ya;
}
}
nedb
  • 586
  • 1
  • 4
  • 12
  • where do you catch `ConcurrentModificationException`, show stacktrace. – alex2410 May 05 '14 at 09:55
  • If a mousePressed event is caught while tick() or render() are being executed an addition attempt will take place while the list is gone through, which is illegal. You need to implement some concurrency safety mechanism. Look for `synchronized`, semaphores... – cangrejo May 05 '14 at 09:57
  • ok, i added the stack trace – nedb May 05 '14 at 09:57
  • Show the codes for methods Particle.tick() and Particle.render(). I think the problem is there. – renz May 05 '14 at 09:58

4 Answers4

3

You have 2 options. If you could, mark the methods tick, render and mousePressed as synchronized. This will solve the issue but though this might not be the best solution if there are insertions at a faster rate.

The other solution would be to change

private ArrayList<Particle> particles = new ArrayList<Particle>(); 

to

private LinkedBlockingQueue<Particle> particles = new LinkedBlockingQueue<Particle>();

This works because, as per documentation of iterator method:

The returned iterator is a "weakly consistent" iterator that will never throw ConcurrentModificationException, and guarantees to traverse elements as they existed upon construction of the iterator, and may (but is not guaranteed to) reflect any modifications subsequent to construction.

Jatin
  • 31,116
  • 15
  • 98
  • 163
2

The simplest thing to do is to lock the collection when you are using it.

synchronized(particles) {
    for(Particle p: particles){
        p.tick();
    }
}

and

Particle p = new Particle(x, y, rand, Game.rand.nextInt(10)-11, new Color(Game.rand.nextInt(1000000000)))
synchronized(particles) {
    particles.add(p);
}

This way you ensure access is not concurrent.

The problem with CopyOnWriteArrayList is that writes are expensive. As the name suggests, it takes a copy of the entire list on every update.

The problem with BlockingQueue is it is not designed for iteration. It will work but not as efficiently.

Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
1

Just to avoid concurent problems and ConcurentModificationException in iterations over collections, you have to use concurrent collection classes.

For ArrayList, for example, you have can try with CopyOnWriteArrayList which is threadsafe variant of ArrayList.

More details about this read here: http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/CopyOnWriteArrayList.html

Bosko Mijin
  • 3,287
  • 3
  • 32
  • 45
0

you are trying to change the internal state of the Array, while looping, this is the expected behavior.

One quick (but comes to some perfomance cost) is to use then CopyOnWriteArrayList, see linked entry, In what situations is the CopyOnWriteArrayList suitable?

Community
  • 1
  • 1
javapapo
  • 1,342
  • 14
  • 26