Frage bzgl. Multithreading

[Klösp]

Hallo,

wir hatte hier die Tage mal ein Thema wo es um synchronized-Blöcke ging.
Hatte mich im nachher noch etwas damit beschäftigt und mir ist eine Frage dazu gekommen.

Hab das Beispiel nochmal sinngemäß nachgebaut:

public class Ablage {
	
	int zettelAnzahl;

	public Ablage(int zettelAnzahl) {
		super();
		this.zettelAnzahl = zettelAnzahl;
	}

	public int getZettelAnzahl() {
		return zettelAnzahl;
	}

	public void setZettelAnzahl(int zettelAnzahl) {
		this.zettelAnzahl = zettelAnzahl;
	}

}

public class Arbeiter extends Thread {

	Betrieb meinBetrieb;

	public Arbeiter(Betrieb meinBetrieb) {
		super();
		this.meinBetrieb = meinBetrieb;
	}

	public Betrieb getMeinBetrieb() {
		return meinBetrieb;
	}

	public void setMeinBetrieb(Betrieb meinBetrieb) {
		this.meinBetrieb = meinBetrieb;
	}

	@Override
	public void run() {
        while( !isInterrupted() )
        {
            meinBetrieb.nehmeZettel(10);
            meinBetrieb.legeZettelab(10);
            try
            {
                Thread.sleep(100);
            }
            catch(InterruptedException e)
            {
                interrupt();
            }
        }
	}

}

package syncBeispiel;

public class Betrieb {
	
	Ablage[] ablagen;
	int anzahlAblagen;

	public Betrieb(int anzahlAblagen, int zettelpAblage) {
		super();
		this.anzahlAblagen=anzahlAblagen;
		this.ablagen = new Ablage[anzahlAblagen];
		for(int i=0; i<ablagen.length;i++){
			ablagen[i]=new Ablage(zettelpAblage);
		}	
	}
	
	public void nehmeZettel(int anzahl){
		Ablage p=ablagen[(int) (Math.random()*this.anzahlAblagen)];
		p.setZettelAnzahl(p.getZettelAnzahl()-anzahl);
	}
	
	
	public void legeZettelab(int anzahl){
		Ablage p=ablagen[(int) (Math.random()*this.anzahlAblagen)];
		p.setZettelAnzahl(p.getZettelAnzahl()+anzahl);
	}
	
	public void pruefen(){
		int sum=0;
		for(Ablage p:ablagen){
			sum+=p.getZettelAnzahl();
		}
		
		System.out.println(sum);	
	}
	
}

public class SyncTest {

	/**
	 * @param args
	 */
	public static void main(String[] args) {
		Betrieb betrieb = new Betrieb(5, 45);
		Arbeiter[] arbeiter = new Arbeiter[50];

		for (int i = 0; i < arbeiter.length; i++) {
			arbeiter[i] = new Arbeiter(betrieb);
			arbeiter[i].start();
		}

		try {
			Thread.sleep(60000);
		} catch (InterruptedException e) {
		}
		
		 for(int i = 0; i < 50; i++)
	        {
	            arbeiter[i].interrupt();
	            try {
					arbeiter[i].join();
				} catch (InterruptedException e) {
					// TODO Auto-generated catch block
					e.printStackTrace();
				}
	        }
		 
		 betrieb.pruefen();

	}

}

Nun soll das Beispiel also einen typischen Büro-Alltag simulieren. Es werden immer ein paar Zettel (10) von einer beliebigen Ablage genommen und dann auf irgendeine andere Ablage verschoben.
Es sollten natürlich keine Zettel abhanden kommen (vieleicht doch nicht so realistisch^^).

Möglichkeit1:
Die Betriebsmethoden werden komplett synchronisiert.

	public synchronized void  nehmeZettel(int anzahl){
		Ablage p=ablagen[(int) (Math.random()*this.anzahlAblagen)];
		p.setZettelAnzahl(p.getZettelAnzahl()-anzahl);
	}
	
	
	public synchronized  void legeZettelab(int anzahl){
		Ablage p=ablagen[(int) (Math.random()*this.anzahlAblagen)];
		p.setZettelAnzahl(p.getZettelAnzahl()+anzahl);
	}

Funktioniert, ist aber nicht gut, weil wenn ein Arbeiter eine Aktion durchführt ist immer der ganze Betrieb lahmgelegt.


Möglichkeit 2:

	public void  nehmeZettel(int anzahl){
		Ablage p=ablagen[(int) (Math.random()*this.anzahlAblagen)];
		synchronized(p){
		p.setZettelAnzahl(p.getZettelAnzahl()-anzahl);
		}
	}
	
	
	public void legeZettelab(int anzahl){
		Ablage p=ablagen[(int) (Math.random()*this.anzahlAblagen)];
		synchronized(p){
		p.setZettelAnzahl(p.getZettelAnzahl()+anzahl);
		}
	}

Sollte besser sein und funktioniert auch soweit.
Allerdings hatte ich zuerst die getter/setter Methoden von Ablage synchronisiert.

	public synchonized  int getZettelAnzahl() {
		return zettelAnzahl;
	}

	public synchronized void setZettelAnzahl(int zettelAnzahl) {
		this.zettelAnzahl = zettelAnzahl;
	}

Das hat dann allerdings nicht funktioniert.
Woran liegt das?

Meine Vermutung:
Würde man den Aufruf der getter/setter so schreiben ist das Problem offensichtlich.

	public void nehmeZettel(int anzahl) {
		Ablage p = ablagen[(int) (Math.random() * this.anzahlAblagen)];
		int number = p.getZettelAnzahl();
		p.setZettelAnzahl(number - anzahl);

	}

	public void legeZettelab(int anzahl) {
		Ablage p = ablagen[(int) (Math.random() * this.anzahlAblagen)];
			int number=p.getZettelAnzahl();
			p.setZettelAnzahl(number + anzahl);
	
	}

Ist also dieser Aufruf das gleiche wie oben?

p.setZettelAnzahl(p.getZettelAnzahl() + anzahl);

Was macht der Compiler daraus, bzw wie wird das zur Laufzeit ausgeführt?
Wann hat der aufrufende Thread den Monitor auf die Ablage und wann wird er freigegeben?

Ist es der Grund für das Problem,dass der Monitor trotzdem zwischen get und set nochmal freigegeben werden kann, oder übersehe ich noch etwas anderes?

Ich hoffe es ist einigermaßen klar geworden was ich wissen möchte. Ist etwas lang geworden.

Vielen Dank im Vorraus

 

[Barista]

Du musst die Abfrage (get) und die Veränderung (set) in einem Block synchronisieren.

 

[Klösp]

Das wäre auch eine Möglichkeit.
Meine Frage war aber mehr wieso es so funktioniert,

    public void  nehmeZettel(int anzahl){
            Ablage p=ablagen[(int) (Math.random()*this.anzahlAblagen)];
            synchronized(p){
            p.setZettelAnzahl(p.getZettelAnzahl()-anzahl);
            }
        }
       
       
        public void legeZettelab(int anzahl){
            Ablage p=ablagen[(int) (Math.random()*this.anzahlAblagen)];
            synchronized(p){
            p.setZettelAnzahl(p.getZettelAnzahl()+anzahl);
            }
        }

mit:

    public int getZettelAnzahl() {
            return zettelAnzahl;
        }
     
        public void setZettelAnzahl(int zettelAnzahl) {
            this.zettelAnzahl = zettelAnzahl;
        }

und so nicht.

    public void  nehmeZettel(int anzahl){
            Ablage p=ablagen[(int) (Math.random()*this.anzahlAblagen)];
            p.setZettelAnzahl(p.getZettelAnzahl()-anzahl);
        }
       
       
        public void legeZettelab(int anzahl){
            Ablage p=ablagen[(int) (Math.random()*this.anzahlAblagen)];
            p.setZettelAnzahl(p.getZettelAnzahl()+anzahl);
        }

mit:

    public synchonized  int getZettelAnzahl() {
            return zettelAnzahl;
        }
     
        public synchronized void setZettelAnzahl(int zettelAnzahl) {
            this.zettelAnzahl = zettelAnzahl;
        }

 

[wef34fewrg]

hey ho,

das sieht ja aus wie das Bankenbeispiel von ein paar Tagen, nicht wahr? Wieso hast du denn nicht gleich da die Frage reingeschrieben?

Erst mal. Falls du keinen bock hast zu warten. Wenn du mal die sleep Methode von 60000 auf 6000 reduzierst, wirst du die Ergebnisse schon sehen.


Was deine Annahme angeht hast du recht.
Die Sperre wird beim Aufruf bzw. beim Verlassen der Methoden gesetzt bzw. aufgehoben. Rufst du also p.setZettelAnzahl(p.getZettelAnzahl()-anzahl); auf, wird die Sperre beim get Aufruf gesetzt und beim Verlassen wieder gelöst. Nachdem die Addition vollzogen wurde, wird die Sperre erst beim set Aufruf wieder gesetzt. In der Zeit kann sich der Kontostand natürlich verändern.

public void nehmeZettel(int anzahl) {
        Ablage p = ablagen[(int) (Math.random() * this.anzahlAblagen)];
        int number = p.getZettelAnzahl();
        p.setZettelAnzahl(number - anzahl);
 
    }

In der Methode kannst du das wunderbar nachvollziehen. Wenn zwei Threads nacheinander die Zeile int number = p.getZettelAnzahl(); aufrufen (z.B. mit dem Ergebnis 20), dann eine Pause einlegen und weiter machen, führt die Subtraktion in beiden Fällen zum Wert 10.Das heisst, 10 Zettel gehen dabei verloren.

Übrigens müsste das Vertauschen der Zeilen
meinBetrieb.nehmeZettel(10);
meinBetrieb.legeZettelab(10);
zu anderen Ergebnissen führen, als die, die du jetzt so bekommst. Wenn du Spaß am Knobeln hast, dann frag dich mal warum das so ist.

P.S. ich hoffe da kommen auch wirklich die Ergebnisse raus, an die ich denke (+/-)



Gruß

 

[Klösp]

Hallo,

vielen Dank für die Hilfe.

Was genau meinst du, was sich verändern soll.
Die Summe bei der unsynchronisierten Version, oder in der synchronsierten Fassung die Anzahl der Zettel auf einer Ablage?
und gib mal nen Tipp...

mfg.

 

[Klösp]

Nochmal eine kleine Ergänzung.

Hab noch ein wenig dran rumgespielt, weil ich das ganze mal mit Locks probieren wollte.
Dazu hab ich das ganze etwas umgebaut.
Das Zettel verschieben macht jetzt der Arbeiter.

import java.util.Random;

public class Arbeiter extends Thread {

	int durchläufe = 0;
	static Random r = new Random();
	Betrieb meinBetrieb;

	public Arbeiter(Betrieb meinBetrieb) {
		super();
		this.meinBetrieb = meinBetrieb;
	}

	public Betrieb getMeinBetrieb() {
		return meinBetrieb;
	}

	public void setMeinBetrieb(Betrieb meinBetrieb) {
		this.meinBetrieb = meinBetrieb;
	}

	public void verschieben(int anzahlZettel) {
		int anzahlAblagen = meinBetrieb.ablagen.length;
		Ablage von = meinBetrieb.ablagen[r.nextInt(anzahlAblagen)];
		Ablage nach = meinBetrieb.ablagen[r.nextInt(anzahlAblagen)];
		boolean b = von.neuberechnen(-anzahlZettel);
		if (b) {
			nach.neuberechnen(anzahlZettel);
		}
	}

	@Override
	public void run() {
		while (!isInterrupted()) {
			// meinBetrieb.nehmeZettel(10);
			// meinBetrieb.legeZettelab(10);
			this.verschieben(10);
			durchläufe++;

			try {
				Thread.sleep(100);
			} catch (InterruptedException e) {
				interrupt();
			}
		}
	}
}

Damit ich das mit Locks machen kann muss ich die eigentliche Berechnung in der Ablage machen, weil ich sonst wieder das alte Problem habe.
Die Möglichkeit ein Lock auf ein bestimmtes anderes Objekt zubekommen wie bei synchronized(object) wird es ja vermutlicht geben, oder?

import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantReadWriteLock;

public class Ablage {

	private int zettelAnzahl;
	private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
	private final Lock readLock = lock.readLock(),
			writeLock = lock.writeLock();

	public Ablage(int zettelAnzahl) {
		super();
		this.zettelAnzahl = zettelAnzahl;
	}

	public int getZettelAnzahl() {
		try {
			readLock.lock();
			return zettelAnzahl;
		} finally {
			readLock.unlock();
		}
	}

	public void setZettelAnzahl(int zettelAnzahl) {
		try {
			writeLock.lock();
			this.zettelAnzahl = zettelAnzahl;
		} finally {
			writeLock.unlock();
		}
	}

	public boolean neuberechnen(int aenderung) {
		try {
			writeLock.lock();
			int check;
			if ((check = zettelAnzahl + aenderung) >= 0) {
				zettelAnzahl = check;
				return true;
			}
			return false;
		} finally {
			writeLock.unlock();
		}
	}
}

Zumindestens im setter muss ich auch noch locken um sicher zu gehen, dass nicht von irgendwo außerhalb dieses Beispiel einer "dazwischen funken" kann.
Dabei kam bei mir eine Frage auf.
Ist Monitor gleich Monitor? Würde es funktionieren wenn ich in der neuberechnen-Methode weiter den Lock nutze und den setter synchronized deklariere?

mfg