ConcurrentModificationException

[Luk10]

Hallo,

Ich schlage mich (wieder) mit einer ConcurrentModException herum ...
Ich weiß, dass diese auftritt wenn ich während einer Schleife, die Elemente einer List über die gelaufen wird, verändere ...

Diese Exception tritt trotz Gegenmaßnahmen auf (d.h. ich erstelle einen neue Collection mit den zu entfernenden Elementen und nach der Schleife rufe ich

removeAll(...)

auf ...

Dennoch:

Exception in thread "AWT-EventQueue-0" java.util.ConcurrentModificationException
at java.util.AbstractList$Itr.checkForComodification(AbstractList.java:372)
at java.util.AbstractList$Itr.next(AbstractList.java:343)
at cells_sim.Gamepanel.paintComponent(Gamepanel.java:91)
at javax.swing.JComponent.paint(JComponent.java:1017)
at javax.swing.JComponent.paintToOffscreen(JComponent.java:5112)
at javax.swing.BufferStrategyPaintManager.paint(BufferStrategyPaintManager.java:278)
at javax.swing.RepaintManager.paint(RepaintManager.java:1220)
at javax.swing.JComponent._paintImmediately(JComponent.java:5060)
at javax.swing.JComponent.paintImmediately(JComponent.java:4870)
at javax.swing.RepaintManager.paintDirtyRegions(RepaintManager.java:803)
at javax.swing.RepaintManager.paintDirtyRegions(RepaintManager.java:714)
at javax.swing.RepaintManager.seqPaintDirtyRegions(RepaintManager.java:694)
at javax.swing.SystemEventQueueUtilities$ComponentWorkRequest.run(SystemEventQueueUtilities.java:125)
at java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:209)
at java.awt.EventQueue.dispatchEvent(EventQueue.java:597)
at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:269)
at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:184)
at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:174)
at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:169)
at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:161)
at java.awt.EventDispatchThread.run(EventDispatchThread.java:122)

Diese Exception hält das Programm nicht auf (es läuft normal weiter, jedoch würde mich schon Interessieren woran das liegt!)

Kann mir jemand dazu Tipps geben?
Hier mal der Code dazu:

for (Sprite sp : Admin.getAdmin().getAllSprites()) {
				if (sp instanceof Moveable) {
					((Moveable) sp).doLogic(delta);
					((Moveable) sp).move(delta);
			}
		}
			if (Sprite.getDeadSprites().size() != 0) {
				Admin.getAdmin().getAllSprites().removeAll(Sprite.getDeadSprites());
				Sprite.setDeadSprites();
			}

- deadSprites ist die Liste mit den zu löschenden Elementen ...
- setDeadSprites: entfernt alle Elemente aus dieser Liste

Kann mir jemand sagen / Tipps geben woran das liegen könnte?

Danke, Luk

 

[Gast2]

Wenn du mit der erweiterten for Schleife über eine Liste läufst kannst du keine Elemente entfernen, deswegen die Exception.
Das Entfernen muss über den Iterator gemacht werden.

 

[Luk10]

Verstehe ich nicht ...

Ich entferne ja nicht während dem Iterieren sonder danach.

Das sollte die Schleife eigentlich schon längst abgeschlossen sein.

Luk

 

[Gast2]

Oh sry, da hat mich die Formatierung etwas irritiert.

Zeig mal die Zeile welche die Exception schmeißt (Gamepanel.java, Zeile 91).

 

[Luk10]

Hier die Methode:

public void paintComponent (Graphics g) {
		super.paintComponent(g);
		
		//g.setColor(Color.blue);
		//g.drawString("fps: "+fps, 0, 10);
		
		for (Drawable draw : Admin.getAdmin().getAllSprites()) {
			draw.drawObjects(g);
		}
	}

Hier die Zeile:

[JAVA=91]for (Drawable draw : Admin.getAdmin().getAllSprites()) {[/code]

EDIT: In diesem Zusammenhang nochmal die Gameloop:
(*Total Verwirrent bin*)

public void run () {
		while (game_running) {
			if (once) {
				try {
					Thread.sleep(100);
				} catch (Exception e) {
					System.out.println("Exception");
				} finally {
					once = false;
				}
			}
			
			setTime();
			
			for (Sprite sp : Admin.getAdmin().getAllSprites()) {
				if (sp instanceof Moveable) {
					((Moveable) sp).doLogic(delta);
					((Moveable) sp).move(delta);
				}
			}
			try { 
				Thread.sleep(20);
			} catch (Exception e) {
				System.out.println("Exception");
			}
			if (Sprite.getDeadSprites().size() != 0) {
				Admin.getAdmin().getAllSprites().removeAll(Sprite.getDeadSprites());
				Sprite.setDeadSprites();
			}
			repaint();	
		}
	}

 

[Bartleby]

Warum machst du es nicht einfach "richtig"?

Iterator<Sprite> iter = Admin.getAdmin().getAllSprites().iterator();
while(iter.hasNext()){
  if ( shoudDelete) {
    iter.remove();
  }
  else{
   //do something else
  }
}

 

[Luk10]

Warum machst du es nicht einfach "richtig"?

Iterator<Sprite> iter = Admin.getAdmin().getAllSprites().iterator();
while(iter.hasNext()){
  if ( shoudDelete) {
    iter.remove();
  }
  else{
   //do something else
  }
}

Weil ich das nicht kenne. :bahnhof:
Wenn ich keine Lösung für meinen Code finde, werde ich mich gerne damit befassen, danke!

 

[Gast2]

Ich schätze mal die Exception tritt auf wenn grad die paintComponent schleife läuft und deine gameLoop removeAll aufruft.

Es könnte evtl. helfen den Zugriff auf die Liste zu synchronisieren.

 

[gman]

Hi,

ändere die Zeilennummern von deinem Beispiel so, das man sieht wo "Gamepanel.java:91" ist. Was mir auffällt:
Sind "Admin" und "Sprite" Variablen oder benutzt du wirklich so viele statische Methoden in deinen Klassen?

- setDeadSprites: entfernt alle Elemente aus dieser Liste

Wenn du damit alle toten Sprites in "Sprite" zurücksetzt, wäre "clearDeadSprites" vielleicht der besser Name.

Edit: da waren andere wohl schneller

 

[Bartleby]

Weil ich das nicht kenne. :bahnhof:
Wenn ich keine Lösung für meinen Code finde, werde ich mich gerne damit befassen, danke!

Das ist aber genau die richtige Möglichkeit, so hat es auch EikeB in Post#2 schon gemeint.

 

[Luk10]

Ich schätze mal die Exception tritt auf wenn grad die paintComponent schleife läuft und deine gameLoop removeAll aufruft.

Es könnte evtl. helfen den Zugriff auf die Liste zu synchronisieren.

Getestet, hilft leider nicht. :noe:

 

[Luk10]

Hi,

ändere die Zeilennummern von deinem Beispiel so, das man sieht wo "Gamepanel.java:91" ist. Was mir auffällt:
Sind "Admin" und "Sprite" Variablen oder benutzt du wirklich so viele statische Methoden in deinen Klassen?



Wenn du damit alle toten Sprites in "Sprite" zurücksetzt, wäre "clearDeadSprites" vielleicht der besser Name.

Admin ist ein Singleton -> statische Referenz
Sprite.deadSprites -> Brauche eine allgemeine Liste ... könnte die auch in Admin packen

clearDeadSprites -> Hast recht ... war noch eine Eclipse automatische setter

 

[Gast2]

Ersetze einfach mal die Schleife in der paintComponent() durch eine Schleife mit iterator (siehe Post von Bartleby). Damit sollte die Exception schonmal nichtmehr geworfen werden.

Allerdings solltest du dir auch nochmal Gedanken machen über die Struktur deines Programms, das sieht alles schon ein wenig merkwürdig aus

 

[Luk10]

Ersetze einfach mal die Schleife in der paintComponent() durch eine Schleife mit iterator (siehe Post von Bartleby). Damit sollte die Exception schonmal nichtmehr geworfen werden.

Allerdings solltest du dir auch nochmal Gedanken machen über die Struktur deines Programms, das sieht alles schon ein wenig merkwürdig aus

Wie mache ich das mit dem Iterator und der Referenz mit der ich drawObjects(g) aufrufe?

Könntest du etas konkreter zu schlechtem Programmierstil werden, der dir aufgefallen ist (Jaja ich weiß Singletons ... wollte aber mal dieses Desingpattern ausprobieren). Würde endlich mal gerne sauber programmieren lernen.

Danke soweit an alle für eure Hilfe, Luk :rtfm:

 

[Gast2]

Ungetestet:

        Iterator<Drawable> iter = data.iterator();
        while (iter.hasNext()) {
            Drawable draw = iter.next();
            draw.drawObjects(g);
        }

Ohne den Kontext zu kennen ist es immer schwer zu sagen wo der Aufbau schlecht ist. Aber diese ganzen statischen Aufrufe kann man sicherlich noch ändern (z.b. Referenzen übergeben).

 

[Luk10]

Wirft die Exception immer noch:
Folgendes Zeile:

Drawable draw = iter.next();

Ich hasse diese Exception ... bringt mich zum Wahnsinn!

 

[Gast2]

Du kannst noch versuchen den Zugriff auf die Liste zu synchronisieren: java.util.concurrent Package mal anschauen, oder http://download.oracle.com/docs/cd/...ections.html#synchronizedList(java.util.List) (hab ich selbst aber noch nie genutzt).

Musst du denn wirklich in beiden Threads auf die selbe liste zugreifen? Kannst du das nicht irgendwie "schöner" lösen?

 

[Marco13]

Nee, is nicht so schlimm. Das "run" läuft in einem Thread, und das paintComponent in einem anderen. Die kommen sich in die Quere. Das allgemeine (einfachste, und etwas an dein Beispiel angepasste) Muster dafür ist sowas wie

class X
{
    // Synchronized machen
    private List<Sprite> sprites = Collections.synchronizedList(new ArrayList<Sprite>());

    // Wird vom Event-Dispatch-Thread aufgerufen
    public void paintComponent(...)
    {
        synchronized (sprites) // Hier synchronizen, wo man drüberläuft
        {
            for (Sprite s : sprites) sprite.paintOrSo();
        }
    }

    // Wird vom Game-Thread aufgerufen
    void run()
    {
        List<Sprite> spritesToRemove = new ArrayList<Sprite>();
        for (Sprite sprite : sprites)
        {
            if (sprite.isDead()) spritesToRemove.add(sprite);
        }
        // Sicher, weil sprites eine synchronized list ist
        sprites.removeAll(spritesToRemove);
    }

Das mit den ganzen Singletons sieht nicht so schön aus. Aber das nur nebenbei...

 

[Luk10]

private List<Sprite> sprites = Collections.synchronizedList(new ArrayList<Sprite>());

Das lässt sich bei mir nich kompilieren ...
Er sagt mir ich könnte nicht auf eine List referenz ein ArrayList-Objekt legen ... geht aber auch nicht mit private ArrayList<Sprite> = Coll....

Aber immerhin verstehe ich das Problem jetzt.

Luk10
:rtfm:

 

[Marco13]

Wenn sich dieser code

import java.util.*;

class Sprite{}

class GehtDoch2
{
    private List<Sprite> sprites = Collections.synchronizedList(new ArrayList<Sprite>());
}

bei dir nicht compilieren läßt, ist irgendwas kaputt...

 

[Marco13]

Ach so moment, kann es sein dass du an irgendwelchen Stellen davon ausgehst, dass "sprites" eine ArrayList ist? Ja, das kommt davon Nimm einfach überall (außer in der oben genannten Zeile) das "Array" vor dem "List" weg...

 

[Luk10]

Sprites ist schon eine Klasse... Und allSprites soll eine ArrayListe sein ...
Hab leider keine Möglichkeit nochmal zu testen ....

Ich meine mich erinnern zu können, dass List nicht mit ArrayList compatibel ist ... (Compiler hat gemeckert...)

So long, Luk10

 

[eRaaaa]

Nächste Vermutung: du hast List aus java.awt. importiert nicht aus util, aber so kommt man ja hier nicht weiter....
Zeige am Besten einfach mal deinen aktuellen Code und/oder nenne die kompletten Fehlermeldungen...

 

[Luk10]

Nächste Vermutung: du hast List aus java.awt. importiert nicht aus util, aber so kommt man ja hier nicht weiter....
Zeige am Besten einfach mal deinen aktuellen Code und/oder nenne die kompletten Fehlermeldungen...

Das wird es sein, glaube ich ... habe das von Eclipse importieren lassen ... muss da wohl noch besser aufpassen.

Danke! Luk

 

[Luk10]

Hmm ich kann's irgendwie nicht lösen ...

Hier mal die Meldung:

Exception in thread "main" java.lang.Error: Unresolved compilation problem:
Type mismatch: cannot convert from List<Sprite> to ArrayList<Sprite>

Luk :rtfm:

 

[eRaaaa]

Weiß jetzt zwar nicht wo du bist, aber das hatte Marco oben doch schon angemerkt!
Nicht direkt ArrayList benutzen sondern immer das Interface auf die linke Seite schreiben!

ArrayList<Sprite> bar = foo(); //foo liefert irgendeine Liste! (List<...>)  <-- FEHLER
List<Sprite> bar = foo();  <--  // kein fehler

 

[Luk10]

Fehler behoben ... lag an eine Methode die nach eine ArrayListe verwendete ...

Danke an alle!

Leider bekomme ich die Exception immer noch gemeldet ... (Programm läuft aber weiter)

 

[Marco13]

Schau' genau an welcher Stelle der Fehler auftritt. Dort muss irgendwo "ArrayList" vorkommen, und das solltest du durch "List" ersetzen können (außer wenn ein "new" davor steht )

EDIT... womit das auch erledigt wäre. Schau' mal nach, an welchen Stellen diese List noch verwendet wird. (Wird bei Singletons schnell unübersichtlich, aber ... ja )

 

[Luk10]

Hier:

public void doLogic(long delta) {
		super.doLogic(delta);
		int index = -1;
		if (delay >= livingTime) {
			for (Sprite sp : Admin.getAdmin().getAllSprites()) {
				if (sp instanceof FoodItem) {
					if (((FoodItem) sp).creatingTime == creatingTime) {
						deadSprites.add(sp);
					}
				}
			}
		}
		else {
			delay += delta;
		}
	}

public void initialize () {
		
		Admin.getAdmin().getAllSprites().add(
		0, new Background(Admin.getAdmin().loadPics(1, Background.BACK_PATH)));
		
		Admin.getAdmin().createCell("minoris");

		Timer t = new Timer(9000, new TListener());
		t.start();
	}

public void actionPerformed(ActionEvent e) {
			Admin.getAdmin().getAllSprites().add(new FoodItem());
		}

 

[Marco13]

Synchronized um die Iteration drumwickeln?

public void doLogic(long delta) {
		super.doLogic(delta);
		int index = -1;
		if (delay >= livingTime) {
                        List<Sprite> sprites = Admin.getAdmin().getAllSprites()
                        synchronized(sprites)
                        {
    			for (Sprite sp : sprites) {
				if (sp instanceof FoodItem) {
					if (((FoodItem) sp).creatingTime == creatingTime) {
						deadSprites.add(sp);
					}
				}
			}
                        }
		}
		else {
			delay += delta;
		}
	}

 

[Luk10]

Ja :noe:

So ein verdammter Mist ...
Zum Glück hält die das Programm nicht an die blöde Exception!

Danke, Luk

 

[Marco13]

Funktioniert's jetzt? ???:L

 

[Luk10]

Kommt drauf an was du mit funktionieren meinst ...

Ich bekomme die Exception immer noch, aber sie hält das Programm nicht an ...

Danke, Luk

 

[Marco13]

Das heißt: Es funktioniert nicht. Hast du um alle Stellen, die über die Liste iterieren, ein synchronized drumgewickelt?

 

[Luk10]

Endlich ... es funktioniert!

Hab jetzt um wirklich alle

synchronized

gewickelt und es klappt!
Vielen vielen danke für deine/eure Hilfe :toll:

Luk10

 

[ice-breaker]

Wenn du auf ein Objekt zugreifst, welches sich mehrere Threads teilen (GameLogic, RepaintManager) dann musst du den Zugriff immer synchronisieren.
Primitiv klappt es wenn du einfach mit synchronized einen wechselseitigen Ausschluss machst, allerdings gibt es da noch weit mehr Möglichkeiten.