Frage zu Synchronized

[Alex04]

Hallo,
wir sind dabei einen Fifo Aspect Springseitig zu implementieren (MethodInvocation). Zu Testzwecken habe ich dies via Proxy nachgebildet. Um die Details geht es aber eigentlich garnicht, sondern eher um die richtige Verwendung des Lock Objekts mit den synchronized Blöcken.
Meine erste Lösung war folgende:

public class FifoServiceInvocationHandler implements InvocationHandler {

      private Object lock = new Object();
      private Object service;
      private Queue<Thread> queue = new LinkedList<Thread>();
      
      public FifoServiceInvocationHandler() {
            service = new MySimpleService();
      }
      
      @Override
      public Object invoke(Object proxy, Method method, Object[] args)
                  throws Throwable {
            System.out.println("Invoked by " + (String)args[0]);
            queue.add(Thread.currentThread());
            synchronized (lock) {
                  try {
                        while(queue.peek() != Thread.currentThread()) {
                             lock.wait();
                        }
                        return method.invoke(service, (String)args[0]);
                  } finally {
                        queue.remove();
                        lock.notifyAll();
                  }
            }
      }
}

Allerdings ist laut Dozent die Anordnung der synchronized Blöcke falsch + die queue.add(...) Methode sollte innerhalb eines synchronen Blocks aufgerufen werden. Die "gesamte FIFO-Funktionalität sei so kaputtgemacht"

Jetzt hätte ich noch folgende Lösung:

public class FifoServiceInvocationHandler implements InvocationHandler {

	private Object lock = new Object();
	private Object service;
	private Queue<Thread> queue = new LinkedList<Thread>();

	public FifoServiceInvocationHandler() {
		service = new MySimpleService();
	}

	@Override
	public Object invoke(Object proxy, Method method, Object[] args)
			throws Throwable {
		System.out.println("Invoked by " + Thread.currentThread().getId() + " command " + (String)args[0]);
		try {
			synchronized (lock) {
				queue.add(Thread.currentThread());
				while (queue.peek() != Thread.currentThread()) {
					lock.wait();
				}
			}
			return method.invoke(service, (String)args[0]);
		} finally {
			synchronized (lock) {
				queue.remove();
				lock.notifyAll();
			}
		}
	}
}

Ich sehe aber nicht wirklich warum das besser sein sollte bzw. was der Unterschied ist...
Außer der Tatsache, dass die Threads schon während der MethodInvocation gequeued werden können....???
Warum sollte queue.add(...) in den synchronized Block?
Und Warum ist bei Variante 1 die Fifo-Funktionalität kaputtgemacht? Laut meinen Tests werden die Threads in richtiger Reihenfolge abgearbeitet... :???

P.S.: Ich weiß nicht, ob Variante 2 richtig ist, wenn Fehler auffallen bitte korrigieren 🙂

Vielen Dank schon mal für die Hilfe!!!

LG
Alex

 

[Ariol]

Wenn das Lock erst nach dem Add kommt wären folgende Szenarien möglich:


gewünscht:

T1 -> add(T1) -> waitForLock -> lock         -> wait -> remove(T1)
T2            -> add(T2)     -> waitForLock                        -> lock -> wait -> remove(T2)

gefährlich:

T1            -> add(T1) -> waitForLock -> lock        -> wait=Endlosscheife
T2 -> add(T2)                           -> waitForLock

 

[SlaterB]

@Alex04
warum verwendest du überhaupt synchronized, wie wäre es denn ganz ohne?
welche Ziele verfolgst du bei dem Einsatz und kannst du nicht auch beantworten warum sie wahrscheinlich mit oder eben ohne snychronisiertes add() erreicht werden?

ich selber könnte jetzt natürlich alles aufschreiben (in der Hoffnung dass ich richtig liege, will ich mal nicht überheblich voraussetzen)
aber wenn du gar nichts schreibst außer fragwürdigen Code, ist da noch bisschen mehr Luft zu Vorarbeit auf deiner Seite

edit:
ok, zu spät 😉 auch wenn es noch zu interpretieren ist, für mich nicht ganz leicht

 

[Alex04]

Hallo und danke schon mal für die Antworten!

Also das ist eine Uni Aufgabe (habe am Montag Prüfung 😉 )
Es geht darum, einen Fifo Aspekt zu basteln, allerdings ohne ReentrantLocks und mit eigenem Lock Objekten.

Mit einem ReentrantLock wäre es sicherlich ziemlich einfach oder:

public class FifoServiceInvocationHandler implements InvocationHandler {

	private Lock lock = new ReentrantLock(true); // true indicates fifo handling
	private Object service;

	public FifoServiceInvocationHandler() {
		service = new MySimpleService();
	}

	@Override
	public Object invoke(Object proxy, Method method, Object[] args)
			throws Throwable {
		System.out.println("Invoked by " + Thread.currentThread().getId() + " command " + (String)args[0]);
		try {
			lock.lock();
			return method.invoke(service, (String)args[0]);
		} finally {
			lock.unlock();
		}
	}
	
}

Wenn ich es richtig verstanden habe liefert mir obiger Code einen Fifo Aspekt. ICh könnte ihn jetzt in Spring per MethodInvocation und AOP-Aspect integrieren usw.
Nun will ich diesen Aspekt aber ohne die Hilfe von Java, sprich mit eigenen Objekten nachbilden. Dafür die beiden Varianten aus Thread 1.

Meine Erläuterung zu den Unterschieden in den jeweiligen Varianten:
Ich verstehe warum queue.add() innerhalb des synchronized Blocks sein muss. Da gemeinsam genutzte Resource, die eben sonst je nach scheduling unterschiedlich befüllt wird (siehe Ariol Problem, danke!).
Als Konsequenz der Tatsache, dass wir die queue.add() Methode in den synchronized Block packen, stimmt jedoch der gesamte Block nicht mehr. Problem ist, dass wir alle Threads außer den ersten der den synchronen Block betritt "suspendieren". D.h. in der Queue ist immer nur der Thread, der als erstes in den synchronized Block gerät. Das ist Zufall, denn notifyAll() weckt alle Threads auf, d.h. irgendeiner kommt in den synchronized Block und fügt sich allein wieder zur Queue hinzu. DIe andern Threads werden wieder suspendiert usw. . Dadurch ist FIFO im endeffekt dann dahin, weil doch wieder random gescheduled wird.
Variante 2 schafft hier Abhilfe, denn nachdem der erste Thread den synchronized Block verlassen hat, werden nach und nach alle andern 1. zur Queue hinzugefügt, 2. schlafend gelegt (wait()) und ermöglichen dadurch das queuen der weiteren Threads...

So viel dazu 🙂

 

[SlaterB]

> Mit einem ReentrantLock wäre es sicherlich ziemlich einfach oder:
ist denn da die Reihenfolge sichergestellt? Frage ich jetzt weil ich das wirklich nicht kenne,

zum Rest:
> Ich verstehe warum queue.add() innerhalb des synchronized Blocks sein muss.
na das war doch deine Hauptfrage, also schon geklärt?
das mit der Reihenfolge hatte ich gar nicht bedacht, wäre alles nur ein großes synchronized, dann wäre das sicher ein Problem,
je nachdem wie FIFO definiert ist, ab welchem Zeitpunkt ist das zwingend, beim Eintritt in die Methode invoke?
strenggenommen könnte ja vor dem ersten Befehl dieser Methode noch ein Threadwechsel stattfinden und ein anderer Thread überholen..,
aber das wären sicher nur ms-Probleme, wer 5 sec später ankommt dürfte keine derartige Chance haben

mit deinem zweiten Code-Block vom ersten Post dürfte jedenfalls FIFO gut umgesetzt sein, dort sehe ich kein Random,
da die lange Bearbeitung des Threads nicht synchronisiert ist sondern immer nur ganz kurze Abschnitte,
ergo langet jeder neuer Thread im Sekundenabstand sofort in der Liste, die Reihenfolge ist gegeben,

> Variante 2 schafft hier Abhilfe,
klingt danach als wäre dir auch das inzwischen (oder schon von Anfang an?) klar, besteht noch irgendeine Frage?

 

[Alex04]

*EDIT
ReentrantLock stellt, sofern mit true im Ctor initialisiert, sicher, dass die Abhandlung der Threads fair, d.h. in der Regel nach FIFO, abgehandelt wird!

Ne es besteht keine Frage mehr, konnte es mir mit euren Hilfen usw. selbst erklären.
Wollte aber keinem meine Erläuterungen vorenthalten, weshalb ich die Erklärung in eigenen Worten nochmal verfasst habe 😉

Also nochmals Danke!