Thread mit Pausefunktion

[noobler]

Ich wollte einen Thread programmieren der von einem anderen Thread aus jederzeit pausiert, wieder aufgeweckt und beendet werden kann. Unten ist das bescheidene Ergebnis zu sehen. Nun zur Frage ob der Code für die beschriebene Aufgabe geeignet ist. Also gibt es problematische Stellen wegen Threadsicherheit oä., kann man das ganze evtl. besser machen?

public class MyThread extends Thread {

    private boolean stopRequested;
    private boolean suspendRequested;
    
    public MyThread() {
		stopRequested = false;
		suspendRequested = false;
		start();
    }
    
    public synchronized void requestStop() {
        stopRequested = true;
        notify();
    }
    
    public synchronized void requestSuspend() {
        suspendRequested = true;
    }
    
    public synchronized void requestWake() {
        suspendRequested = false;
        notify();
    }
    
    private synchronized boolean continueThread() {
        while(!stopRequested && suspendRequested) {
            try {
                wait();
            }
            catch(InterruptedException e) {
                // check flags again
            }
        }
        return !stopRequested;
    }
    
    @Override
    public void run() {
        while(continueThread()) {
            // do stuff
        }
    }
}

 

[SlaterB]

finde ich gut, Zeile 7 + 8 braucht man nicht, sonst wüßte ich erstmal nix zu ändern

requestSuspend() müsste evtl. nichteinmal synchronisiert sein, bei boolean sehe ich nichts, was schiefgehen kann,
die anderen Methoden brauchen das allein schon für wait + notify

 

[Marco13]

Hui, ich komme gerade von Angelika Langer's Seiten zu "volatile & Co" - ich würde das synchronized NICHT weglassen

 

[Gast2]

Angelika Langer's Seiten zu "volatile & Co"

hilf mal bitte Unwissenden auf die Sprünge - URL oder so wären nicht schlecht

 

[Marco13]

AngelikaLanger.com - An Introductory Glance at the Java Memory Model (JMM) - Angelika Langer Training/Consulting und unten sind die 5 weiteren Teile verlinkt - habe es ehrlich gesagt nur überflogen, aber es hat gereicht, damit ich weiß (dass ich nichts weiß und ) dass man, um fundierte Aussagen dazu machen zu können, wann man welches "volatile" braucht um welches "synchronized" weglassen zu können, erstmal einiges :rtfm: müßte ....

 

[Ebenius]

Ich würde nicht gegen [c]this[/c] sondern gegen einen privaten Member synchronisieren; wait/notify entsprechend. Falls Du das aus einem bestimmten Grund nicht willst, solltest Du notifyAll() statt notify() rufen.

Zeile 32 würde ich ersetzen: [java=32]interrupt(); // check flag again
break;[/code]

Zeile 40 entsprechend: [java=40] while(continueThread() && !isInterrupted()) {[/code]

Ansonsten find ichs auch gut.

Ebenius

 

[javimka]

Ich würde stopRequested und suspendRequested volatile machen, so gehst du auf Nummer sicher.

 

[noobler]

Viele dank für eure Hilfe! Ich habe mal versucht eure Verbesserungsvorschläge umzusetzen:

public class MyThread extends Thread {

	private Object monitor;
    private boolean stopRequested;
    private boolean suspendRequested;
    
    public MyThread() {
		monitor = new Object();
		start();
    }
    
    public void requestStop() {
		synchronized(monitor) {
			stopRequested = true;
			notifyAll();
		}
    }
    
    public void requestSuspend() {
		synchronized(monitor) {
			suspendRequested = true;
		}
    }
    
    public void requestWake() {
		synchronized(monitor) {
			suspendRequested = false;
			notifyAll();
		}
    }
    
    private boolean continueThread() {
		synchronized(monitor) {
			while(!stopRequested && suspendRequested) {
				try {
					wait();
				}
				catch(InterruptedException e) {
					interrupt(); // check flag again
					break;
				}
			}
			return !stopRequested;
		}
    }
    
    @Override
    public void run() {
        while(continueThread() && !isInterrupted()) {
            //do stuff
        }
    }
}

Leider hapert es jetzt mit dem Verständnis.
1. War "gegen einen privaten Member synchronisieren" so gemeint?
2. Welchen Vorteil hat das Synchronisieren gegen einen privaten Member?
3. Die Änderungen mit interrupt() und isInterrupted() dienen dazu den Thread zusätzlich von "aussen" durch interrupt() beenden zu können, oder? Wenn der Thread in wait() hängt und von aussen requestWake() aufgerufen wird, wird dann nicht auch jedesmal eine Exception erzeugt, die dazu führen würde, dass der Thread durch den aufruf von requestWake() beendet wird?
4. "Volatile" hab ich so gar nicht verstanden. Was hätte die Verwendung von Volatile für Vorteile? Und könntest du das an einer Methode konkretisieren?

 

[Ebenius]

Alle notifyAll()-Aufrufe müssen nun [c]monitor.notifyAll()[/c] heißen; alle wait()-Aufrufe entsprechend [c]monitor.wait()[/c] entsprechend.

1. War "gegen einen privaten Member synchronisieren" so gemeint?
2. Welchen Vorteil hat das Synchronisieren gegen einen privaten Member?

1. Ja, 2. Da Du den Thread wiederverwendbar implementierst, kann jeder andere von außen auch auf die Thread-Variable synchronisieren, darauf warten, etc. und dem Mechanismus so ins Handwerk pfuschen. Wenn Du synchron zu einem separaten Monitor arbeitest kannst Du diese Fehlermöglichkeit von vorn herein ausschließen.

3. Die Änderungen mit interrupt() und isInterrupted() dienen dazu den Thread zusätzlich von "aussen" durch interrupt() beenden zu können, oder?

Es ging mir nicht darum, zu ermöglichen, den Thread auch von außen über interrupt zu beenden. Es geht mir eher darum, dass Thread nunmal die interrupt-Methode hat und MyThread diese erbt. Daher sollte das Verhalten auch dem Erwarteten Verhalten eines 08/15-Thread entsprechen.

Wenn der Thread in wait() hängt und von aussen requestWake() aufgerufen wird, wird dann nicht auch jedesmal eine Exception erzeugt, die dazu führen würde, dass der Thread durch den aufruf von requestWake() beendet wird?

Verstehe ich nicht.

4. "Volatile" hab ich so gar nicht verstanden. Was hätte die Verwendung von Volatile für Vorteile? Und könntest du das an einer Methode konkretisieren?

Meiner Meinung nach keine Vorteile, da Du ohnehin synchron arbeitest und arbeiten solltest.

Den Monitor würde ich noch [c]final[/c] machen; weil's besser ist.

notifyAll() musst Du nicht benutzen, kannst Du aber. Das musst (solltest) Du nur machen, wenn Du riskieren musst, dass mehere Threads auf den Monitor warten. Da Dein Monitor nun von außen nicht mehr erreichbar ist, ist sicher gestellt, dass das nicht passiert.

Ebenius

 

[javimka]

Wenn du einen internes Lock-Objekt monitor hast, dann muss du auch auf diesen wait() und notifyAll() aufrufen. Also [c]monitor.wait()[/c].

Variabeln, die als volatile deklariert wurden, sind für alle laufenden Threads "gleich". Zurecht fragst du dich, wann sie denn nicht gleich sind. Der Computer verfügt über zig-Zwischenspeicher, in denen dein boolean gespeichert ist. Der Wert wird dabei aus dem RAM in einen Cache gespeichert, wenn er oft verwendet wird. Aber nicht alle Threads müssen zwingend mit derselben Kopie arbeiten. Es kann passieren, dass ein Thread "seine" Kopie der boolean auf true stellt, ein anderer Thread aber eine andere Kopie verwendet, die von dem zunächst nichts mitkriegt. Natürlich wird im praktischen Gebrauch früher oder später jeder Thread, der diese boolean liesst, die Veränderung sehen, aber theoretisch kann das auch eine lange Zeit dauern. Wenn du sie aber als volatile deklarierst, ist jeder Thread gezwungen, zu jedem Zeitpuntk den echten Wert der Variable zu verwenden. Konkret wird die Variable dann stets aus dem RAM gelesen, so wird jeder schreibende Zugriff sofort von allen Threads wahrgenommen.

Sieht dann so aus:

    private volatile boolean stopRequested;
    private volatile boolean suspendRequested;

 

[Ebenius]

Sieht dann so aus:

    private volatile boolean stopRequested;
    private volatile boolean suspendRequested;

Nur damit's nicht unter geht; da alle Zugriffe synchronisiert sind, trifft dieser Zustand nie auf und daher sollten die Variablen auch nicht [c]volatile[/c] deklariert sein.

Ebenius

 

[noobler]

Hier nochmal der überarbeitete Code. Falls noch jemand einen Verbesserungsvorschlag oder eine elegantere Lösung für das selbe Problem hat, darf das kommentiert werden.

public class MyThread extends Thread {

    private final Object monitor;
    private boolean stopRequested;
    private boolean suspendRequested;
    
    public MyThread() {
        monitor = new Object();
        start();
    }
    
    public void requestStop() {
        synchronized(monitor) {
            stopRequested = true;
            monitor.notify();
        }
    }
    
    public void requestSuspend() {
        synchronized(monitor) {
            suspendRequested = true;
        }
    }
    
    public void requestWake() {
        synchronized(monitor) {
            suspendRequested = false;
            monitor.notify();
        }
    }
    
    private boolean continueThread() {
        synchronized(monitor) {
            while(!stopRequested && suspendRequested) {
                try {
                    monitor.wait();
                }
                catch(InterruptedException e) {
                    interrupt();
                    break;
                }
            }
            return !stopRequested;
        }
    }
    
    @Override
    public void run() {
        while(continueThread() && !isInterrupted()) {
            //do stuff
        }
    }
}

Zu 2.: Ich hab das ein paar mal getested, läuft augenscheinlich so wie beabsichtigt. Der Thread lässt sich mit interrupt() und requestStop() beenden. Also vielen Dank soweit.

Allerdings hab ich noch eine kleine Frage:
Wenn ich in der Hauptschleife wie folgt wait(1000) benutze bekomme ich eine "llegalMonitorStateException".

while(continueThread() && !isInterrupted()) {
	try {
		wait(1000);
		System.out.println("working");
	}
	catch(Exception e) {
		e.printStackTrace();
	}
}

Benutze ich stattdessen sleep(1000) verhält es sich wie erwartet.

while(continueThread() && !isInterrupted()) {
	try {
		sleep(1000);
		System.out.println("working");
	}
	catch(Exception e) {
		e.printStackTrace();
	}
}

Woran liegt das?
Und letztlich nochmal zum Thema volatile:
Wenn ich den Zugriff auf die Flags synchronisiere hat es doch den gleichen Effekt wie volatile?
Ist volatile dann nur eine verkürzte Schreibweise?
Man deklariert einfach ein Variable als volatile und kann mit beliebig vielen Threads lesend und schreibend auf sie zugreifen?
Volatile bewirkt das der lesende und schreibende Zugriff auf eine Variable tatsächlich eine atomare Operation ist?

 

[javimka]

Du machst es ja sowieso synchronized, dann kannst du volatile weglassen.

 

[SlaterB]

wenn auf monitor synchronisiert wird, dann monitor.wait() + monitor.notify()

 

[Ebenius]

Alle notifyAll()-Aufrufe müssen nun [c]monitor.notifyAll()[/c] heißen; alle wait()-Aufrufe entsprechend [c]monitor.wait()[/c] entsprechend.

Ebenius