2

I am currently working on a little project for school which is a TowerDefense game.

When initializing the game, it starts a Thread that calculates everything. After starting that Thread, my code jumps into a loop, that loops my graphics code. I think that it is using 2 Threads then. One thread is for calculating and the other one is the "standard" Thread, that is used when you open the program itself.

Now I wanted to add Projectiles to my Towers. For that I need to add a new Projectile Object to my Entity ArrayList. and whenever I try to do so it says

Exception in thread "main" java.util.ConcurrentModificationException

and points to the line where my graphicsloop is, that tries to loop my Entity Array. That makes sense, because my graphicsloop tries to read while my Projectiles are added to this Array. I tried to work with "synchronized" and I think I know how to use it and what it is for, but Im not that sure where I would use it in my code. I try to solve this problem since yesterday and couldnt find anything on the Internet how to solve it. Also Im not sure what piece of Code helps you, if you didnt understand exactly how my program works and what the problem is so if you need anything, feel free to ask.

Would be awesome, if someone could help me solving this problem and I already want to thank you, if you are still reading

edit: I think these are the important pieces of code:

World update method:

private void updateRunning() {
    waveTimer();
    for (Entity e : entities) {
        if (e.isMonster() && !block) {
            e.toMonster().move();
            if (e.toMonster().getHealth() <= 0)
                entities.remove(e);
        } else if (e.isTower()) {
            e.toTower().setTarget(e.toTower().findTarget());
            e.toTower().shoot();
        } else if (e.isProjectile()) {
            e.toProjectile().update();
        }
    }
}

Renderer loop method:

private void updateRunning() {
    cam.update();
    for (Entity ent : world.getEntities()) {
        ent.draw();
    }
    cursor.update();
    cursor.draw(); // Must always be on bottom draw-methods, to be drawed
                   // on top of
                   // everything else
    cam.refresh();
}

edit2

private void addWave() {
  if (wave % 3 == 1) {
    for (int i = 0; i < 6; i++) {
      entities.add(new Monster(this, i + 1, path.getCoord(0).x,  
                   path.getCoord(0).y - i * 57, 0.35f, Assetloader.monster01));
    }
  } else if (wave % 3 == 2) {
  } else {
  }
}

private void attack(Monster target, float damage) {
  world.addEntity(new Projectile(world, vec.x, vec.y, 20, 20, target,  
                this, Assetloader.cursor01));
  ready = false;
  lastShot = System.currentTimeMillis();
}

public void addEntity(Entity e) {
  this.entities.add(e);
}
besplash
  • 346
  • 1
  • 5
  • 17
  • not to the point but helpful , read about THREAD LOCAL – Hussain Akhtar Wahid 'Ghouri' May 26 '14 at 14:38
  • 1
    It's hard to say what's going on without seeing some code. But what I think is going on, is that one thread tries to acces your list while an other thread is already iterating over that list. So, perhaps you'll have to use locks to give your threads concurrent access to the list. What you can try to use are list interfaces designed for this, such as CopyOnWriteArrayList or ConcurrentHashMap . – Joetjah May 26 '14 at 14:42
  • 1
    Show your game loop please. Usually `java.util.ConcurrentModificationException` happens when you are removing stuff while iterating over it. That doesn't mean you are accessing things from different threads. – Thomas Jungblut May 26 '14 at 14:47
  • added both loops now and as you can see Im iterating simultaneously – besplash May 26 '14 at 14:58
  • @besplash see, nothing with threads. Just have a look at: http://www.javacodegeeks.com/2011/05/avoid-concurrentmodificationexception.html or my answer ;) – Thomas Jungblut May 26 '14 at 15:07

5 Answers5

2

When you need to collect data in several threads, you should use a structure that supports concurrency. In Java, such classes can be found inside java.util.concurrent package. For your case, you should use CopyOnWriteArrayList class instead of ArrayList.

From my experience, using an ArrayList or a concurrent support variation of ArrayList is not the best option for these cases. It would be better using a concurrent queue like ConcurrentLinkedQueue.

Luiggi Mendoza
  • 85,076
  • 16
  • 154
  • 332
  • Thanks a lot ! This one actually seems to work. My Projectiles arent flying at the moment, but that may be a different problem. Atleast my frames are fine and not missing any objects as sometimes before and I get no Exceptions. I let you know, if it solved my problem, when my Projectiles work with this solution – besplash May 26 '14 at 15:05
0

By the looks of your problem, it seems that you are sharing a data structure between many threads. The appropriate class for that is java.util.Vector, since it is (more or less) a synchronized version of an ArrayList

Jorge_B
  • 9,712
  • 2
  • 17
  • 22
  • 1
    Since we have not seen the code, it may be just the appropriate situation. Say they only need to add objects to the list, not checking it before nor doing anything more complicated. Of course, in any other situation, Jon is right in that matter and they would need some synchronized block or something like that – Jorge_B May 26 '14 at 14:44
  • I don't need to see the code to know that `Vector` is an option. – Luiggi Mendoza May 26 '14 at 14:47
  • Tried to just replace my ArrayList with Vector, but it doesnt work so how exactly do I have to change my Code? I think it has something to do with your (more or less) – besplash May 26 '14 at 14:48
  • 1
    `Vector` won't prevent a `ConcurrentModificationException` when one thread inserts into the vector while another thread is iterating over it. That is exactly what the OP is experiencing. – Kenster May 26 '14 at 14:49
  • @besplash If replacing it has not removed your problems, then maybe your situation is more complex as LuiggiMendoza and Kenster point in their comments. Have a look at your code and think of the critical sections of it, and implement synchronized blocks to make sure that they get executed in mutual exclusion – Jorge_B May 26 '14 at 14:50
0

You can use a CopyOnWriteArrayList from the concurrent collections' package to automatically synchronize access.

Beware, the internal array is copied every write as the name implies, so this can be challenging performance-wise.

Otherwise, you may want to go a bit more low-level and experiment with a ReentrantReadWriteLock.

Mena
  • 47,782
  • 11
  • 87
  • 106
0

If you would want to use synchronized you would have to do something like that

synchronized(myList) {
    //Do stuff
}

So no other thread can access the list while another Thread is in the synchronized block. But watch out for deadlocks

Here you have a tutorial on synchronized http://tutorials.jenkov.com/java-concurrency/synchronized.html

wastl
  • 2,643
  • 14
  • 27
0

Just use the iterator feature to remove an item while iterating:

private void updateRunning() {
        waveTimer();
        Iterator<Entity> iterator = entities.iterator();
        while(iterator.hasNext()) {
            Entity e = iterator.next();
            if (e.isMonster() && !block) {
                e.toMonster().move();
                if (e.toMonster().getHealth() <= 0)
                    iterator.remove(e);
            } else if (e.isTower()) {
                e.toTower().setTarget(e.toTower().findTarget());
                e.toTower().shoot();
            } else if (e.isProjectile()) {
                e.toProjectile().update();
            }
        }
    }

This has nothing to do with threads, you just can't remove elements from a list while iterating over it using the for-each loop.

Thomas Jungblut
  • 20,854
  • 6
  • 68
  • 91