Robert
Robert

Reputation: 233

Do I correctly shutdown these simultaneous threads

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

Answers (3)

mxns
mxns

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

Abdul
Abdul

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

AlexR
AlexR

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

Related Questions