ConcurrentModificationException trotz synchronized?

[Rock Lobster]

Servus,

mit "synchronized" stehe ich irgendwie auf Kriegsfuß, es funktioniert nie so, wie ich es will.
Wenn ich die folgenden drei Methoden habe (die alle synchronized sind):

		public synchronized void addTaskListener(TaskListener l)
		{
			_taskListeners.add(l);
		}
		
		public synchronized void removeTaskListener(TaskListener l)
		{
			_taskListeners.remove(l);
		}
		
		protected synchronized void sendMessage(TaskMessage msg)
		{
			for(TaskListener l : _taskListeners)
				l.eventOccured(msg, this);
		}

Kann es dann trotzdem zu einer ConcurrentModificationException in der sendMessage() geben? Eigentlich nicht, oder? Weil bei mir trat vorhin ein paarmal eine auf (leider nur selten). Aber ich hab keine Ahnung, wie ich es umschreiben müßte, damit es endlich funktioniert ???:L

 

[SlaterB]

schaue dir die Variable _taskListeners an,
ist diese private oder öffentlich, gar statisch?
greift außer diesen 3 Operationen noch jemand darauf zu, wird sie irgendwo als Rückgabewert rausgegeben?

 

[Rock Lobster]

Ja, private ist sie, also es macht nur die Klasse selbst was damit (und das sind nur die oberen Methoden)

 

[SlaterB]

tja, und testen/ nachbauen kannst du es nicht,
schlechte Voraussetzungen

bist du dir zumindest mit der Interpretation der Exception sicher?

baue vielleicht mal

try {
  for(TaskListener l : _taskListeners) {
      try {
            l.eventOccured(msg, this); 
      }catch (Throwable t) {
        // Ausgabe a
      }
   }
}catch (Throwable t) {
    // Ausgabe b
}

um ganz sicher zu sein, dass es nicht einfach ein Fehler bei der Event-Ausführung ist

 

[Rock Lobster]

Okay mittlerweile kommt's sehr oft. Sobald ich den Aufruf auf removeListener wegnehme, läuft alles wie geschmiert (logisch).

Hab es so gemacht wie Du es gesagt hast, es fliegt die "äußere" Exception. Hier der Stracktrace:

java.util.ConcurrentModificationException
	at java.util.HashMap$HashIterator.nextEntry(Unknown Source)
	at java.util.HashMap$KeyIterator.next(Unknown Source)
	at de.*****.task.TaskListener$TaskListenerSender.sendMessage(TaskListener.java:74)
	at de.*****.task.TaskExecutor$TaskThread.run(TaskExecutor.java:106)

 

[SlaterB]

also mir fällt dazu nix weiter ein,
ich halte Java für narrensicher, irgendwas muss bei dir noch sein

public class Test
{
    private List<TaskListener> _taskListeners = new ArrayList<TaskListener>();

    public synchronized void addTaskListener(TaskListener l)
    {
        _taskListeners.add(l);
    }

    public synchronized void removeTaskListener(TaskListener l)
    {
        _taskListeners.remove(l);
    }

    protected synchronized void sendMessage(TaskMessage msg)
    {
        for (TaskListener l : _taskListeners)
            l.eventOccured(msg, this);
    }

    public static void main(String[] args)
        throws Exception
    {
        final Test t = new Test();
        for (int i = 0; i < 1000; i++)
        {
            Runnable r = new Runnable()
                {
                    public void run()
                    {
                        try
                        {
                            TaskListener l = new TaskListener();
                            t.addTaskListener(l);
                            Thread.sleep((int)(Math.random() * 50));
                            t.sendMessage(new TaskMessage());
                            Thread.sleep((int)(Math.random() * 50));
                            t.removeTaskListener(l);
                        }
                        catch (InterruptedException e)
                        {
                            e.printStackTrace();
                        }
                    }
                };
            new Thread(r).start();
            Thread.sleep((int)(Math.random() * 10));
        }
    }
}


class TaskMessage
{
    static int count;
    int number;

    public TaskMessage()
    {
        this.number = count++;
    }
}


class TaskListener
{
    static int count;
    int number;

    public TaskListener()
    {
        this.number = count++;
    }

    public void eventOccured(TaskMessage msg, Test test)
    {
        System.out.println("Listener " + number + ", verarbeite Message " + msg.number);
    }
}

läuft

 

[Marco13]

also mir fällt dazu nix weiter ein,
ich halte Java für narrensicher, irgendwas muss bei dir noch sein

(code)
....

läuft

Wie das bei Thread-Problemen so ist: Die Programme funktionieren, außer an Donnerstagen bei Vollmond.

Aber an den Fragesteller: Schau vielleicht mal hier
http://java.sun.com/j2se/1.5.0/docs/api/java/util/Collections.html#synchronizedSet(java.util.Set)
die synchronisation sollte überall auf das gleiche Objekt erfolgen - bei dir wäre das "this", aber bei einem Zugriff von außen sollte es vielleicht eher die Collection selbst sein

 

[Rock Lobster]

Hab es mit dem SynchronizedSet versucht, aber auch hier tritt die Exception auf. Das darf doch echt nicht wahr sein!
Ein anderer Versuch war, die Methoden nicht synchronized zu machen, aber dafür synchronized-Blöcke in den Methoden zu verwenden, die alle auf ein eigenes Lock-Objekt synchronizen. Aber auch das ging nicht.

Dauernd hab ich diese verdammten Probleme mit synchronized... aber so falsch kann mein Code doch gar nicht sein...

 

[Rock Lobster]

Oh mann...
ich weiß an was es liegt

Wenn ein Event auftritt, wird sendMessage() aufgerufen. Diese wiederum ruft eventOccured() auf, und in eventOccured() habe ich in meinem Fall die removeTaskListener() aufgerufen. Also wird letztendlich der TaskListener innerhalb der for-Schleife entfernt, und daß das nicht gut geht, ist eigentlich klar.

Jetzt weiß ich nur nicht, wie ich das am besten lösen soll, aber das ist ja auch wieder 'ne ganz andere Frage... ich werd mir was einfallen lassen

 

[SlaterB]

verdammt, darauf hätte ich kommen müssen,
die einzig logische Alternative

eine umständlich sichere Möglichkeit:
bei sendMessage() ein Flag setzen,
in remove (und in add?) bei gesetzten Flag nicht die Liste berühren sondern den Listener in einer zweiten Liste vermerken,
am Ende von sendMessage() flag entfernen, nach der zweiten Liste schauen und nochmal remove() für diese Listener aufrufen,

oder etwas einfacher: generell beim remove (udn add?) die zu entfernenden Listener gesondert speichern und nur zu Beginn von sendMessage übernehmen,

wenn aber letztlich sogar sendMessage 2x eineinander aufgerufen wird, dann hilft nur noch ein Flag
und dann ist z.B. das Einfügen + Senden gar nicht möglich, solange die erste sendMessage()-Operation die Liste blockiert,

also wird nie richtig gut

 

[HoaX]

wieso nicht einfach die liste klonen und über den klon interieren?

 

[SlaterB]

oder so
(wobei dann synchronized selber etwas seltsam wirkt, falls es nicht ausschließlich der Abwehr der Exception gilt)

 

[Rock Lobster]

Die Idee ist gut. Rein theoretisch müßte ich synchronized ja dann auch komplett weglassen können, oder?