Stop thread from other class

Question

i'm trying to stop a specific Thread that implement Runnable.

I already try a few things that i found while googling it, but still can't find the best answer.

I have two Class.

1st is Timer. This class will create a countdown. It will send a result if countdown == 0 to another class.

public class Timer implements Runnable {

public Timer(int countdown, Room room, String cmdName) {
    this.room = room;
    this.countdown = countdown;
    this.cmdName = cmdName;
}

public void setResultThread(IResultThread resultThread) {
    this.resultThread = resultThread;
}

@Override
public void run() {

    for (int i = countdown; i >= 0; i--) {

        countdown--;
        if (i == 0) {
            resultThread.setResult(true);
        }

        try {
            TimeUnit.MILLISECONDS.sleep(1000);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
    }
}
}

2nd is User. I want to stop the Thread from this class.

    Thread thread;
    String threadName = "player-" + user.getName();
    thread = utils.getThreadByName(threadName);

    if (thread != null) {
        thread.stop(); // i used stop before, but i read its not a good way
        thread.interrupt();
    }

    if (nextTurn == 4) {
        // do something
    } else {

        int countDown = 10
        //getting player name

        if (playerName != null) {
            String newThreadName = "player-" + playerName;

            Timer timer = new Timer(countDown, room, Send.PLAYER_TIMER.toString());
            thread = new Thread(timer, newThreadName);
            timer.setResultThread(resultThread);
            thread.start();
        }
    }

I'm not sure if there's something wrong with my code or something. I tried catch the exception, but the Thread countdown is still running. Its seems like its interrupt while the Thread is sleep. I ever tried with volatile boolean, but I don't think is a good way to approach in this case and of course its not working either.

I used thread.stop() before, but its not a good idea. This is the 1st time I'm using Thread, and its still a little bit confused for me.

I will appreciate any suggestion and answer. Thanks alot.

---

ANSWER

I put an answer just in case there's someone have a same problem with me.

@Override
public void run() {

    try {
        while (countdown >= 0) {

            TimeUnit.MILLISECONDS.sleep(1000);
            countdown--;
            if (countdown == 0) {
                resultThread.setResult(true);
            }
        }
    } catch (InterruptedException e) {
        countdown = 0;
        Thread.currentThread().interrupt();
        //e.printStackTrace();
    }
}

Answer 1

The preferred way, IMO, to stop your Timer is to have eg. a cancel method

public class Timer implements Runnable {
  ...
  public void cancel() {
    countdown = 0;
  }
  ...
}

Then replace thread.stop() with timer.cancel(), obviously you would need a reference to timer or have Timer extend Thread instead of implementing Runnable.

And as Andy said, in your run() method it should be

run() {
  ...
  try {
    for () {
      ...
    }
  } catch (...
}

Answer 2

There is no need to call Thread.stop().

The reason why your code doesn't currently stop when you interrupt the thread is that the handling of the interruption occurs inside the loop:

for (int i = countdown; i >= 0; i--) {
  // ...
  try {
    // Sleep
  } catch (InterruptedException e) {
  }
}

When the thread is interrupted during (or before) the sleep, the interruption is "swallowed": you handle it, so the loop just continues on to the next iteration, if any.

There's a Robin Williams skit about British police, who, owing to their lack of guns, have to say "Stop! Or.... I'll say stop again!". If you kept calling interrupt, eventually your thread would finish, because eventually the loop would execute the appropriate number of times. But this would be really messy, and is quite unnecessary.

Reverse the order of the loop and the try/catch:

try {
  for (int i = countdown; i >= 0; i--) {
    // ...

    // Sleep
  }
} catch (InterruptedException e) {
}

Now, when the interruption occurs, execution moves to after the loop, so no more loop iterations occur.

(Of course, you could leave your code as it is, and just return from the catch; but multiple return points can make the code harder to reason about)

---

You should also consider re-interrupting the current thread when you catch the InterruptedException:

// ...
} catch (InterruptedException e) {
  Thread.currentThread().interrupt();
}

This sets the interrupted flag on the current thread (confusingly, this is orthogonal to InterruptedException), allowing any code calling the Runnable to know that the thread was interrupted.

It's not strictly necessary to do this: if you know that the code is at the top of a thread's call stack (like here: there is no "caller" to be made aware of interruption). But re-interrupting the thread is nearly always the right thing to do (*), and doesn't actually hurt at all in this case.

(*) An example of when you wouldn't is if you are rethrowing the InterruptedException, or if you are writing threading framework code, and you know for sure it is the right thing not to.

Answer 3

The correct way to stop a thread from another class is to interrupt the running thread. Then within the thread you must detect the interruption and then end the thread execution.

don't call thread.stop, also there is no need to use volatile variables

for example

public class StoppingThreadFromOtherClassExample {

    public static void main(String[] args) throws InterruptedException {
        StoppingThreadFromOtherClassExample stoppingThreadFromOtherClassExample = new StoppingThreadFromOtherClassExample();
        stoppingThreadFromOtherClassExample.doTheWork();
    }

    private void doTheWork() throws InterruptedException {

        Thread myThread = new Thread(new MyClassA());
        myThread.start();

        Thread.sleep(1000);
        myThread.interrupt();

        myThread.join();

    }
}

class MyClassA implements Runnable {

    int counter = 0;

    @Override
    public void run() {
        while (true) {
            counter++;
            System.out.println(counter);
            if (Thread.interrupted()) {
                System.out.println("this thread was interrupted. it will stop now!!");
                break;
            }
        }
    }
}

Answer 4

You can give each thread a name and whenever you want to close the thread just iterate through the collection looking up for the exact name of the thread and call the stop.

To get the list of running thread

get an iterable set:

Set threadSet = Thread.getAllStackTraces().keySet();