How to completely stop a Thread which includes a buffered reader?

Question

It's a "guess the number in 10 seconds game", looping until the user wants the program to stop (the game restarts indefinitely, or until the user types "exit"). The problem is, if a round is failed (time out), and then the user prints the correct answer, the game says it's won. That means, the thread of the last game (round) was not interrupted. Here are the classes:

import java.util.Random;
import java.util.logging.Level;
import java.util.logging.Logger;
import static java.lang.Math.abs;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;


public class Initializer extends Thread {

    @Override
    public void run() {

        Random rand = new Random();
        int x = (abs(rand.nextInt())) % 101;
        GameEngine gameEngine = new GameEngine(x);
        gameEngine.start();
        synchronized (this) {
            try {
                wait(10000);
            } catch (InterruptedException ex) {
                Logger.getLogger(Initializer.class.getName()).log(Level.SEVERE, null, ex);
            }
        }
        if (gameEngine.isAlive()) {
            gameEngine.interrupt();
            System.out.println("Time is up! The number was " + x);
        }
        try {
            Thread.sleep(5000);
        } catch (InterruptedException ex) {
            Logger.getLogger(Initializer.class.getName()).log(Level.SEVERE, null, ex);
        }
    }
}



public class GameEngine extends Thread {

    private final int valueToGuess;

    public GameEngine(int x) {
        this.valueToGuess = x;
    }

    @Override
    @SuppressWarnings("ConvertToTryWithResources")
    public synchronized void run() {
        BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
        while (true) {
            try {
                int x;
                String line = br.readLine();
                if(line.equalsIgnoreCase("exit")) System.exit(0);
                try {
                    x = Integer.parseInt(line);
                } catch (NumberFormatException e) {
                    System.out.println("Please enter a number!");
                    continue;
                }
                if (x < valueToGuess) {
                    System.out.println("Too low!");
                }
                if (x > valueToGuess) {
                    System.out.println("Too high!!");
                }
                if (x == valueToGuess) {
                    System.out.println("You won!");
                    break;
                }
            } catch (IOException ex) {
                Logger.getLogger(GameEngine.class.getName()).log(Level.SEVERE, null, ex);
            }
        }
        try {
            br.close();
        } catch (IOException ex) {
            Logger.getLogger(GameEngine.class.getName()).log(Level.SEVERE, null, ex);
        }
        this.notifyAll();
    }
}


public class Main {

    public static void main(String[] args) throws InterruptedException {
        System.out.println("Guess the number!");
        Initializer counter = new Initializer();
        counter.start();
        while (true) {
            if(counter.isAlive()) continue;
            System.out.println("Game starts in:");
            Thread.sleep(1000);
            System.out.println("5");
            Thread.sleep(1000);
            System.out.println("4");
            Thread.sleep(1000);
            System.out.println("3");
            Thread.sleep(1000);
            System.out.println("2");
            Thread.sleep(1000);
            System.out.println("1");
            Thread.sleep(1000);
            System.out.println("Guess the number!");
            counter = new Initializer();
            counter.start();
        }
    }
}

So if I run it, considering the value to guess to be 50, and I fail the round, when the next round starts, if I type 50, I get "You won!" from the last round, then "Wrong" from the current round. I chose BufferedReader over Scanner because I read that Scanner.nextLine() is blocking the thread, making it uninterruptable, while BufferedReader does not block it. The problem must be when I notify the Initializer Thread to interrupt the GameEngine Thread. I'm new to multithreading so it must be a mistake with all those synchronized or wait/notify instructions. Any help please?

Answer 1

The main problem behind it's not working is your wrong understanding about Thread.interrupt method. So, Let's talk about it.

When you call interrupt on any thread object, an internal flag known as interrupt status is set. You can read about it more here. So when you are calling gameEngine.interrupt(); interrupt status flag gets set, but you are not checking the status of interrupt flag anywhere in your code. So your thread executes as normally it would do.

Now, when it reaches String line = br.readLine(); line. It waits for the user input and when user enters the correct answer you just displayed, it matches with the if (x == valueToGuess) condition and your loop terminates due to break inside that if condition.

So what you need is a way to check whether the interrupt flag is set or not and that you can check using Thread.interrupted() or thread.isInterrupted() method.

The main difference between these two the methods is that, Thread.interrupted() method is static method and by default it checks for the current thread and it will clear the interrupt status flag. Whereas thread.isInterrupted() is an instance method and can be called on an instance of thread to check whether it is interrupted or not. It will not clear the interrupt status flag.

Now, solution to your problem. String line = br.readLine(); is the only blocking call in your code. So add the following line after it.

So it would be something like

String line = br.readLine();
// This will check the interrupt status flag and if set will clear it. So next call will return false.  
if (Thread.interrupted()) {  
    break;
}

or

String line = br.readLine();
// This will check the interrupt status flag and will not clear it, so any following call will also return true.
if (this.isInterrupted()) {    // or just if (isInterrupted()) as you are extending the Thread class. 
    break;
}

Hope it helps.

Answer 2

The program is not working the way you expect it to is primarily because of your understanding of how Thread.interupt works. Consider the following two lines of code :

   if (gameEngine.isAlive()) {
        gameEngine.interrupt();
        System.out.println("Time is up! The number was " + x);
   }

Your expectation is that just by calling interupt, you will be halting the GameEngine thread on which interrupt is called. This is not true. The interrupt method will only update the state of the Thread as interrupted. An InterruptedException will then be thrown by any method that monitors the interrupt state of the Thread.

Since you don't call any such method in your GameEngine thread, an InterruptedException will never be thrown. Try adding a method that monitors the interrupt state (such as sleep) inside the run method of GameEngine and you will have the desired effect of an InterruptedException being thrown.

Sidenote : Always prefer composition over inheritance. Don't extend from Thread unless you want to modify the way Thread class performs some functionality. Implement Runnable instead.