Deadlock durch synchronized (JPanel)

Hiho,

ich stehe gerade vor einem Synchronisierungsproblem.

Ich habe eine Datenstruktur BinaryTree. Die Knoten eines BinaryTrees haben Koordinaten. Jeder Knoten wird durch einen PaintedNode repräsentiert und in Abhängigkeit der Koordinaten eines Knotes wird der zugehörige PaintedNode auf einem JPanel positioniert

Während der Animation werden durch einen Thread die Koordinaten der Knoten verändert, daraufhin aktualisiere ich die Position der PaintedNodes. Hierzu habe ich die Methode updatePaintedNodes(BinaryTree tree); die rekursiv den Binärbaum durchläuft um alle Knoten zu updaten.
Wird das Fenster auf dem das JPanel sitzt in der Größe verändert, wird auch updatePaintedNodes aufgerufen (vom EDT? nicht sicher, hab hierfür keinen eigenen Thread erstellt), um die die PaintedNodes neu auszurichten.

Wenn ich nun die Fenstergröße verändere während eine Animation im Gang ist, kommt es zu Fehlberechnungen. Deswegen wollte ich die updatePaintedNodes() Methode nun synchronized setzen. Wenn ich das allerdings mache und während einer Animation die Fenstergröße verändere, friert mein Programm sporadisch ein. Ich tippe mal auf einen Deadlock kann mir aber nicht erklären wieso, da doch synchronized genau das verhindern sollte?!

Hier nochmal grob die Methode

public synchronized void updatePaintedNodes(BinaryTree tree){

		//lese Knoten aus tree aus
                //suche dazu passenden PaintedNode
                //aktualisiere Position von PaintedNode 

		if(tree.getLeftSon() != null){
			updatePaintedNodes(tree.getLeftSon());
		}
		if(tree.getRightSon() != null){
			updatePaintedNodes(tree.getRightSon());
		}
}

edit: Die Methode befindet sich in dem JPanel, welches auch das Runnable Interface implementiert. Heißt, der Animationsthread arbeitet auf dem JPanel und beim Verändern der Fenstergröße wird auch auf dem JPanel gearbeitet
Übersehe ich da irgendwas ganz simples? Rekursion von synchronized Methoden sollte kein Problem darstellen, wegen Reentrancy…ich komm nich drauf

ein einzelnes synchronized im Quellkode kann nur in recht konstruierten Fällen ein Problem sein,
hier mit Aufruf nur innerhalb des eigenen Objektes denke ich gar nicht

gibt es noch mehr synchronized-Methoden dieser Klasse, gibt es andere von dir synchronisierte Klassen?
suche Projekt nach ‚synchronized‘ ab

falls es mehrere gibt, dann wird das Zusammenspiel interessant,

in jedem Fall lohnt sich im Zweifel der mühsame Weg in das Logging:
alle ‚synchronized‘ an Methoden usw. sind auszuradieren, nur normale synchronized brauchbar, dann

public void updatePaintedNodes(BinaryTree tree){
  String tn = Thread.currentThread().toString();
  System.out.println("Methode .., Thread .. steht vor dem Monitor ..");
  synchronized (this) {
    System.out.println("Methode .., Thread .. ist 'drin' ");
.....
    System.out.println("Methode .., Thread .. geht gleich raus ");
  }
  // vor dieser Meldung könnten vielleicht schon andere Threads wieder rein,
  // deswegen die Meldung zuvor
  System.out.println("Methode .., Thread .. ist 'draußen' "); 
}

kann man noch beliebig ausbauen, statt System.out.println() eine Klasse mit dynamischen Log-Daten die man besser nach bestimmten Monitor oder Thread sortieren kann,
diese Klasse könnte auch nachverfolgen und vorhersagen, ob schon ein Lock auf Monitor X besteht (wenn dieser immer übergeben wird, alle Aktionen gemerkt),
automatisch manche Deadlocks erkennen…

na, ich schweife wohl von deinem Problem auf eine allgemeine Lösung ab, die ich selber auch nicht habe :wink:
vielleicht machen manche Debugger das auch schon

Ein Runnable JPanel kommt mir irgendwie merkwürdig vor. Implements drückt ja immerhin eine “ist ein”-Beziehung aus. Es ist nur eine Ahnung und ich weiß, dass man sich damit in Foren zurückhalten sollte, aber ein Runnable JPanel, welches evtl. über seine Eigenschaft als Swing-Komponente mit dem EDT zusammenspielt… klingt für mich nach einer möglichen Ursache. Habe den Quellcode von JPanel und seinen Oberklassen deshalb mal nach synchronized Blöcken durchforstet. Es gibt dort diverse syncrhonized Blöcke und sogar syncrhonized Methoden.

Ich habe keine weiteren synchronized Methoden oder synchronized Blöcke in meinm Projekt, das fällt schon mal weg.
Hab deinen Tipp befolgt und einen synchronized Block eingesetzt, anstatt die ganze Methode synchronized zu machen, leider ohne Erfolg.
Die Sache wird aber immer skurriler. Selbst wenn ich nur die Animation laufen lassen, ohne dabei die Fenstergröße zu verändern “friert” mein Programm schon im synchronized-Block (/Methode, macht keinen Unterschied) ein.

Obwohl das nach Deadlock aussieht, kann ichs absolut nicht nachvollziehen wie der zustande kommt, da eben zum Zeitpunkt des Einfrierens nur ein Thread auf die Methode zugreift (sagen mir die println’s) und es nur diesen einen synchronized Block gibt

@ nillehammer: Stimmt. Daran, dass da noch synchronized-Blöcke/Methoden vorkommen können, hab ich gar nicht gedacht

stimmt, eine JPanel-Klasse oder so sollte das tatsächlich besser nicht sein,
dann hast du keine Kontrolle, was noch alles synchronisiert ist,
kann man ja leicht umbauen, Ansatz genug

funktioniert es in diesem Fall wieder, wenn synchronized weg ist?
vielleicht hast du inzwischen einen ganz anderen Fehler eingebaut, Endlosschleife durch falsche Verknüpfung

generell wäre interessant, ein gewisses Log in die Methode einzubauen,
wer immer diese startet, geht der nach bestimmter Zeit wieder raus?

wenn nein, dann der Fehler ‚eigentlich‘ darin zu suchen sein, etwa Endlosschleife zwischen paar Knoten,
wenn ja, sollte der Rest ‚eigentlich‘ funktionieren

Was genau veranlasst nicht zu der Annahme, dass es ein Deadlock ist? In Eclipse kann man das ganze übrigens im Debugger starten, und sollte dann recht schnell rausfinden können, WO genau er hängt. (Geht aber auch mit anderen Debugger, VisualVM wohl auch…)

Folgendes hab ich herausgefunden, vllt könnt ihr mir helfen das zu interpretieren, da ich nicht ganz durchsteige. Dazu muss ich aber nochmal ein bisschen ausholen und die Funktion nochmal genauer beschreiben:

Ich habe folgende Klassen:

BinaryTree - Binärbaum
AbstractTreeNode - Wert eines Knotens im Binärbaum, Binärbaum besteht also aus 1 oder mehreren AbstractTreeNodes
PaintedNode - Erbt von JComponent, visualisiert einen AbstractTreeNode

public synchronized void updatePaintedNodes(BinaryTree tree){
 
       AbstractTreeNode tNode = tree.getValue() //Hole Knoten aus BinaryTree
       PaintedNode pNode = paintedTree.get(tNode) //paintedTree ist eine Map<AbstractTreeNode, PaintedNode> die AbstractTreeNodes mit PaintedNodes verknüpft
       pNode.setBounds(tNode.getX(), tNode.getY(), width, height); //hier entsteht der Freeze. Width und Height spielen keine Rolle. Die Koordinaten von tNode wurden schon aktualisiert
 
        if(tree.getLeftSon() != null){
            updatePaintedNodes(tree.getLeftSon());
        }
        if(tree.getRightSon() != null){
            updatePaintedNodes(tree.getRightSon());
        }
}

Dass mein Programm an dieser Stelle einfriert hab ich per Debugger herausgefunden und mit weiteren println’s validiert.
Anschlißend hab ich in der Klasse PaintedNode setBounds() überschrieben um den Hänger weiterzuverfolgen. Der Freeze ensteht beim Aufruf super.setBounds() (weiter kann ich in der Hierarchie mit dem Debugger nicht gehen?!).
Diesen Aufruf habe ich dann weiter in der Vererbungshierarchie nach oben verfolgt: In der setBounds der Klasse Component wird reshape aufgerufen. Und reshape hat einen synchronized-Block auf das final Object LOCK in der Klasse.

Kann es daran liegen? Auf den ersten Blick denke ich nicht. Der Aufruf, den ich bis in setBounds() verfolgt habe, wird vom Animationsthread aufgerufen. Somit hält dieser den LOCK auf meinem JPanel in dem gezeichnet wird und sollte auch den LOCK in allen höhren Ebenen der Verberungshierarchie bekommen.
Bei dem was ich gerade beschrieben habe, lief nur die Animation. Die Fenstergröße wurde nicht verändert

solche Prozesse sind doch unabsehbar,

updatePaintedNodes() blockiert das JPanel und WILL den Lock auf eine der anscheinend untergeordneten JComponents,
vielleicht kommt gerade das Zeichnen oder sonst ein interner Vorgang vorbei, HAT den Lock auf eine der untergeordneten JComponents und will von dort auch ins JPanel,
so warten beide, bis in Ewigkeit

es ist nach wie vor vollkommen unnötig, eine synchronized-Methode auf einem JPanel zu haben,
da kann man eigentlich auch aufhören,


aber da ich eh mich selber gerne reden höre :wink: :

vielleicht schaffst du das auszugeben, was sich unter ‚java thread dump‘ in Suchmaschinen finden läßt,
darin müsste dann zum Blockade-Zeitpunkt stehen, was alle Threads machen, in welche Methode sie sich gerade befinden

oder mit Thread.getAllStackTraces() eine Map aller Thread und insbesondere deren StackTraces holen,
diese manuell ausgeben,
je Thread per Schleife von allen Elementen Klassenname, Methodename usw.,

lasse dafür einen Thread separat laufen, stelle am Anfang eine bestimmte Zeit ein (sleep) ab welcher die Ausgabe passieren soll,
oder mit System.in.read() auf eine bestimmte Tastatur zum Start warten steht sicherlich auch noch zur Verfügung

Ziel: was macht der AWT-Thread zum Problemzeitpunkt?

Hmja, wenn es da um irgendwelche Components (und sowas wie setBounds) geht, wäre mein erster Gedanke auch der “TreeLock”, der globale Lock von allen GUI components, gewesen. Ganz lapidar gesagt: Synchronisation sollte dort nicht notwendig sein, weil solche Änderungen sowieso nur vom Event-Dispatch-Thread gemacht werden dürfen. (Und wenn man DOCH synchronisiert, lockt’s halt ggf. irgendwo).

Du könntest/solltest/müßtest “updatePaintedNodes” ggf. auf dem Event-Dispatch-Thread aufrufen, dann sollte das erledigt sein (aber natürlich müßte man mehr über den Rest des Programmes wissen, um sagen zu können, ob dann an anderer Stelle irgendeine Synchronisation nötig ist).

Ganz klar ist das von mir unglücklich gelöst, dass mein JPanel Runnable implementiert. Da habe ich eindeutig einen Fehler beim Entwurf gemacht.
Aber alles nochmal umzubauen würde mich Zeit kosten die ich im Moment nicht habe.

Aber mal anders gedacht: Hätte ich die Runnable-Sache in ne eigene Klasse auslgelagert, würde ich von da aus updatePaintedNodes() aufrufen. Würde das dann nicht zum gleichen Ergebnis führen?

btw: Danke für eure bisherige Hilfe

Wie kann ich denn gezielt updatePaintedNodes auf dem EDT ausführen lassen? Der Animationssthread verändert die Koordinaten der AbstractTreeNodes. Irgendwo müssen die Änderungen ja auch an die PaintedNodes übertragen werden? Den Aufruf in die paintcomponent packen?

Es geht ja nicht um die Klasse, sondern um den Thread, der die Methode ausführt. Wo und wann wird denn “updatePaintedNodes” aufgerufen? Eventuell könntest du als schnellen Hack den Aufruf

void mainLoop()
{
    doThis();
    doThat();
    ...
   updatePaintedNodes();
   ...
}

einfach ändern in

void mainLoop()
{
    doThis();
    doThat();
    ...
    SwingUtilities.invokeLater(new Runnable()
    {
        @Override 
        public void run()
        {
            updatePaintedNodes();
        }
    });
    ...
}

aber das ist schon sehr “pragmatisch”…

Die updatePaintedNodes() wird an 3 Stellen aufgerufen:

  • Während der Animation in der der Thread die Koordinaten verändert
  • in ancestorResized() wenn die Größe des Fensters verändert wird um die PaintedNodes neu auszurichten
  • in updatedPaintedNodes selber, da rekursiv

Der Aufruf in der Animation müßte dann entsprechend so eingewickelt werden. Oder anders: Wenn bei

public void updatePaintedNodes(BinaryTree tree){
    System.out.println("Calling it on "+Thread.currentThread());
    ...

etwas anderes steht als ~“AWT Event Queue”, dann ist das nicht gut…

Allerdings muss man auch aufpassen, dass der EDT nicht mit Arbeit “zugemüllt” wird, wenn das (aus dem Animations-Thread heraus) ZU oft auf den EDT gelegt wird. Aber es gibt eben viele Dinge, die man berücksichtigen muss, deswegen ist es schwierig, etwas zu benennen, was “die Patentlösung” ist…

Nochmal Danke für eure Anregungen. Hab die Methode jetzt dem EDT übergeben.