Reputation: 233
As in the subject do I correctly shutdown these simultaneous threads?
I assigned a volatile field and check it repeatedly in while loop.
Is there alternative way to do it(like using synchronize or wait() method), please show me.
EDIT I edited code. Is there any way of checking if Thread is alive by different method thatn isAlive();? Perhaps:
boolean isAlive(){
return running;
}
import javax.swing.JOptionPane;
public class Wat extends Thread {
private char c;
private int interv;
private volatile boolean running = true;
Object synchObj;
public Wat(char c, int interv) {
this.c = c;
this.interv = interv;
synchObj = new Object();
}
public void run() {
while (running) {
synchronized (synchObj) {
try {
showChar(c);
synchObj.wait(interv * 100);
} catch (InterruptedException e) {
e.printStackTrace();
}
}
}
}
public synchronized static void showChar(char c) {
System.out.println(c);
}
public void shutdown() {
running = false;
synchronized (synchObj) {
synchObj.notify();
}
}
public static void main(String[] args) throws InterruptedException {
Wat w1 = new Wat('A', 3);
Wat w2 = new Wat('B', 4);
Wat w3 = new Wat('C', 5);
w1.start();
w2.start();
w3.start();
Object[] options = { "Shutdown A", "Shutdown B", "Shutdown C" };
int option;
while (w1.isAlive() || w2.isAlive() || w3.isAlive()) {
option = JOptionPane.showOptionDialog(null,
"Which one would you like to shut?", "Threads",
JOptionPane.YES_NO_CANCEL_OPTION,
JOptionPane.QUESTION_MESSAGE, null, options, options[2]);
switch (option) {
case JOptionPane.YES_OPTION:
w1.shutdown();
break;
case JOptionPane.NO_OPTION:
w2.shutdown();
break;
case JOptionPane.CANCEL_OPTION:
w3.shutdown();
break;
}
Thread.sleep(1);
}
}
}
Upvotes: 2
Views: 130
Reputation: 199
Your code will probably work fine, but the Thread.sleep is not very elegant. I would do something along these lines, calling the shutdown() method to quit the thread
Object synchObj = new Object();
public void run() {
while (running) {
synchronized (synchObj) {
try {
System.out.println(new Date());
synchObj.wait(5000);
} catch (InterruptedException e) {
// error handling
}
}
}
}
public void shutdown() {
running = false;
synchronized (synchObj) {
synchObj.notify();
}
}
public static void main(String[] args) throws InterruptedException,
IOException {
ThreadTest test = new ThreadTest();
test.start();
BufferedReader tReader = new BufferedReader(new InputStreamReader(
System.in));
tReader.readLine();
test.shutdown();
}
EDIT added test code
Upvotes: 4
Reputation: 331
It seems the program is perfect, uses volatile boolean variable and makes it false.
synchronized need only multi thread access. instead of sleep you can use wait, it has a object access rather than static sleep, also any time you can interrupt the waiting.
Upvotes: 0
Reputation: 115328
Yes, you are closing the threads correctly. The only comment is that you are breaking the encupsulation here because the flag running
is accessed directly. I'd recommend you to add method shutdown()
that changes this flag to false
.
EDIT.
I have just noticed that you are calling sleep()
inside the loop. This is indeed bad practice in most cases. You should probably call wait(timeout)
. In this case your shutdown()
method will change the flag to false
and then call notify()
on same monitor. This will cause your thread to exit immediately.
Upvotes: 3