# ConcurrentModificationException Aktion aussetzen



## Apollo4 (25. Aug 2012)

Hallo,

in meinem game gibt es einen Thread, der alle Gegner in der Map bewegt.
Ich bekomme manchmal eine Fehlermeldung ConcurrentModificationExceptionerscheint, ich vermute wenn Thread und Spieler gleichzeitig auf ein Objekt zugreifen.

Alle Objekte der Map befinden sich in eine Hashtable, hatte erst HashMap gedacht, aber da Hashtable syncron ist mich für diese entschieden.

Wie sage ich, das der Thread einmal aussetzen soll, wenn Objekt belegt, oder wie gehe ich da am besten vor?

MfG
Apollo


----------



## tfa (25. Aug 2012)

ConcurrentModificationException fliegt, wenn du eine Collection veränderst, während du mit z.B. einem Iterator auf sie zugreifst (das muss nicht unbedingt ein Threading-Problem sein). Um das zu verhindern, könnte man eine Kopie der Collection anlegen und über diese iterieren. Oder du musst den Ablauf besser synchronisieren. 
Die Vorstellung, überall synchronized davor zu schreiben oder statt HashMap Hashtable zu nehmen, und damit alle Threading-Probleme zu lösen, ist leider falsch.


----------



## Apollo4 (25. Aug 2012)

ok, ich kopiere jetzt mit clone() die Hashtable bevor ich alle Monster 1x bewege. Dabei durchläuft er mit einer for-each die ganzen Einträge. Da aber die keys und values nicht geklont werden kann es immer noch zu einem Fehler kommen, da der Spieler ein Gegner töten kann, während er diese bewegen will / die kopierte Hashtable durchläuft?


----------



## tfa (25. Aug 2012)

Richtig. Das Bewegen und Töten muss dann eben entsprechend synchronisiert werden.


----------



## Apollo4 (25. Aug 2012)

Bin mir nicht sicher wie man das am besten sycronisieren kann. Da Gegner und Monster ja sich separat von einander  bewegen (key event, thread). Habe jetzt die Monster bewegungen, solange eine Spieler bewegung im gang ist angehalten. Allerdings kann ich das bei dem Spieler ja nicht machen. Da bräuchte ich so eine Methode schaue in ein paar Millisekunden nach ob  Object in use.

Ist so etwas möglich?


----------



## Luk10 (25. Aug 2012)

Anderer Vorschlag, den ich immer verwende um diese Mist-Exception zu vermeiden:


```
public void render() {
		for (int i = 0; i < renders.size(); i++) {
			if (renders.get(i).isDead()) {
				renders.remove(i--);
				continue;
			}
			renders.get(i).render(renderer);
		}
	}
```

Ziemlich selbsterklärend. Währende des Iterierens über die (Array)List wird auf isDead geprüft. Wenn ja entferne ich und gehe in die nächste Iteration. Wenn nicht: rendern!

Bin mir aber nicht ganz sicher ob das jetzt dein Problem behebt.


----------



## sophismo (26. Aug 2012)

Nicht aussetzten - geschlossene Logik ist das gesuchte Ziel.
Zerbrich dir den Kopf, wie du den Spieleablauf so weit "steuern" kannst, dass Zugriffe nur noch seriell möglich sind.

Qualxis Tutorial hat genau das selbe aufgebracht.. die Lösung des Tuts ist allerdings nicht formvollendet - also hin und wieder hab ich das trotzdem bekommen. Aber nachdem man ja keine Herausforderung scheut, wenn man programmiert, hab ich mir ein geschlossenes System überlegt:

http://www.java-forum.org/spiele-mu...5-quaxli-2d-spiele-tutorial-8.html#post933599

Also Tutorial anschauen, und dann meinen Kommentar lesen, falls du was nicht verstehst.


Kurz:
3 Collections (bei mir Arrays<Sprite>) 1 Acteure, 1 Buffer, 1 Printer.
Alle "Erstelle Sprite X" Methoden haben als Ziel den Buffer. So wird sichergestellt, dass jede Logik - also erstellen einer Explosion nach einer Kollision nicht auf die Akteure Kollektion zugreift. Und dann, bevor du deine Sprites bewegst, machst du eine Methode SpillBuffer, die den Buffer ausleert und den Inhalt an die Akteure anhängt. Bewegen ist ja sicher^^. Und für paranoide (wie mich) wird zum Printen die Actor Collection noch einmal gecloned.

Aber eine dumme Frage - da das mit den Hastables - wie genau iterierst du da? Also wenn du alle bewegen willst zB.? hash.hasNext() oda so gibts ja nicht. Will mir die api ersparen.


----------



## Spacerat (26. Aug 2012)

Wenn man innerhalb eines Threads Collections bearbeitet bzw. drüber iteriert, sollte man stets bei jedem Durchlauf eine Kopie erstellen und nicht mit dem Original arbeiten. Während man kopiert muss ein Monitor auf das Original gesetzt werden, damit dort weder Elemente entfernt noch hinzugefügt werden können. Danach kann der Thread gemütlich über die Kopie iterieren, während das Original an anderen Stellen wieder modifiziert werden darf. Wenn ein GameObjekt das Zeitliche gesegnet hat, muss es entsprechend markiert werden, damit andere Threads es schnell überspringen können, wenn es noch einen Platz in deren Kopie ergattert hat. Ich würde aber Luk10s Methode dahingehend abändern, dass der Thread die Elemente nicht entfernt, weil bei dieser Methode wäre das eh' überflüssig.

```
for(GameObject go : objectsCopy) {
  if(!go.isDead()) {
    go.doWork();
  }
}
```


----------



## sophismo (27. Aug 2012)

Klingt einleuchtend, nur eine Frage hab ich: wenn die kopie dann ein neues Objekt erstellen will, dann in diesem Fall ws im Original (Kopie wird ja grade bearbeitet)? Dann ist aber die Methode mit dem Buffer schneller, weil der ja keine gesamte Kopie braucht, sondern nur neue Objekte. Zum zeichnen gibts auch bei mir eine Kopie, aber das wäre glaub ich gar nicht nötig (in meinem Fall).


----------



## Spacerat (27. Aug 2012)

Wer immer neue Objekte hinzufügen oder entfernen darf, arbeitet logischerweise am Original. Meistens macht das der Controller. Ein Thread, der Objekte erstellen will, teilt dies also einem solchen mit. Prinzipiell geht's nur darum, dass Creator/Destroyer (CD) und User verschiedene Collections benutzen, wobei jeder User seine eigene Kopie des Originals erstellt und der CD das Original behandelt.


----------



## Gregorrr (27. Aug 2012)

Übrigens, kannst du eine 
	
	
	
	





```
ConcurrentSkipListMap
```
 nehmen, dann kannst du darüber iterieren, wie du möchtest (single-threaded, multi-threaded) - die ist nämlich thread-safe. Dafür ist natürlich nicht garantiert, dass die iteration auch die neuesten werte beinhaltet, bspw. kann die map nach Erzeugung des Iterators verändern worden sein.


----------



## Spacerat (27. Aug 2012)

@Gregorrr: Das scheint mir eine relativ überflüssige Erfindung zu sein. Die im Thread kopierte Collection ist ja nur im Thread bekannt und muss deswegen auch nicht synchronisiert sein. Ebenso das Original, welches nur dann synchronisiert sein muss, wenn es von den Threads kopiert wird. Die Threads müssen sich die benötigten Collections ja nicht mal selber kopieren. Man kann den Controller mit diversen [c]getSpezielleItemCollection()[/c]-Methoden ausstatten, welche dann [c]Collections.unmodifiableCollection(spezielleItems)[/c] zurück gibt. Die Threads wissen dann sofort was sie dürfen und was nicht. Ob die Collection anschliessend verändert wird oder nicht, darf den Thread pro Durchlauf auch nicht interessieren.


----------



## sophismo (27. Aug 2012)

Spacerat hat gesagt.:


> Wer immer neue Objekte hinzufügen oder entfernen darf, arbeitet logischerweise am Original. [...] Prinzipiell geht's nur darum, dass Creator/Destroyer (CD) und User verschiedene Collections benutzen, wobei jeder User seine eigene Kopie des Originals erstellt und der CD das Original behandelt.



Ich glaube du redest komplett an mir vorbei.
Nach dem, was du gesagt hast, müsste der CD ja alle Veränderungen (nicht nur löschen und erstellen) im Original umsetzten. 
Was mich stört, ist die Tatsache, dass auch die einzelnen Aufrufe der Kopie Elemente erstellen können, wobei dann andauernd von jedem zugreifenden Objekt zuvor eine Kopie angelegt wird, was zu mindesten 2-3 kompletten Collections führt. Mit einem Buffer werden nur neue Objekte "hinangestellt", bis sie vom Controller ganz normal verarbeitet werden. Das klingt für mich wie die gleiche Strategie, 
nur anders umgesetzt.
Zu guterletzt gehts nicht nur darum, dass User und Cd separat zugreifen, sondern auch Selbstzugriffe innerhalb einer Kopie verhindert werden. (Bei einer Kollision entsteht eine Explosion - müsste schon in Kopie Nr 3, bzw das Original unter 2 anderen (user,cd)).

Aber war lehrreich, Danke.


----------



## Apollo4 (27. Aug 2012)

Danke für eure Antworten

Habe das im moment noch so das die "tote" Objekte direkt aus der liste gelöscht werden. Wenn der über die clone liste iteriert, wäre es doch egal ob er isDead prüft oder ob der Eintrag nicht mehr existiert also "if (entry.getKey()!=null)" oder?


----------



## Spacerat (27. Aug 2012)

sophismo hat gesagt.:


> Ich glaube du redest komplett an mir vorbei.
> Nach dem, was du gesagt hast, müsste der CD ja alle Veränderungen (nicht nur löschen und erstellen) im Original umsetzten.
> Was mich stört, ist die Tatsache, dass auch die einzelnen Aufrufe der Kopie Elemente erstellen können, wobei dann andauernd von jedem zugreifenden Objekt zuvor eine Kopie angelegt wird, was zu mindesten 2-3 kompletten Collections führt. Mit einem Buffer werden nur neue Objekte "hinangestellt", bis sie vom Controller ganz normal verarbeitet werden. Das klingt für mich wie die gleiche Strategie,
> nur anders umgesetzt.
> ...


Wieviele Kopien es von den Collections gibt ist eigentlich egal. Wichtig ist nur, dass es keine DeepCopys sein dürfen, also halt Listen, die dieselben Instanzen des Originals enthalten. Und wenn ein Thread nur hinzugefügte Objekte benötigt, dann müsste die "getSpecialItemCollection"-Methode in diesem Fall "getAddedItems()" heissen. Sowas würde sich aber auch anders lösen lassen, z.B. über das Observer-Pattern. Dann könnte der entsprechende Thread seine eigene Collection selbst aktualisieren, wenn ihn der Controller über jedes hinzugefügte bzw. entfernte Objekt informiert. Aber was könnten das für Dinge sein, die ein Controller beim Hinzufügen oder Entfernen von Objekten nicht selbst machen könnte bzw. müsste?


----------



## Sophismatic (28. Aug 2012)

Nicht beim Entfernen, aber die Kollisionserkennung muss zB die eine Liste öffnen und dann bei Kollisionen zB. neue Explosionen erstellen,.. egal, das passt schon .
Hab deine Vorgehensweise nun endlich kapiert, und das ist eigentlich nicht so verschieden, wie die meine. Nur hab ich gedacht, dass es rechenintensiver wird, wenn man mehrere Klone hat.

Zum Entfernen:
Ich hab einem jeden Sprite ein "Alive" boolean flag gegeben. Jede Runde wird einmal die Liste durchgekramt und wenn alive=false, dann remove! Also mehr brauchst du natürlich nicht prüfen.

Ja, für die Klon Liste kann es egal sein (kommt drauf an, ob du die Liste dann wieder ins Original bringst, das wäre nicht gut, sonst = wurscht).


----------

