24

In the below code, i have a while(true) loop. considering a situation where there is some code in the try block where the thread is supposed to perform some tasks which takes about a minute, but due to some expected problem, it is running for ever. can we stop that thread ?


public class thread1 implements Runnable {

    /**
     * @param args
     */
    public static void main(String[] args) {
        // TODO Auto-generated method stub
        thread1 t1 = new thread1();
        t1.run();

    }

    @Override
    public void run() {
        // TODO Auto-generated method stub
        while(true){
            try{        
                Thread.sleep(10);

            }
            catch(Exception e){
                e.printStackTrace();
            }
        }
    }
}
Riduidel
  • 22,052
  • 14
  • 85
  • 185
randeepsp
  • 3,572
  • 9
  • 33
  • 40
  • 6
    In Windows: Task Manager, end the `javaw` process. :) – Buhake Sindi Jun 20 '11 at 11:58
  • @The Elite Gentleman What are you talking about? It only works for Windows users, that's not a multi-platform way to do it, and thus, it's not very Java :oP – SteeveDroz Jun 20 '11 at 12:01
  • 8
    Not directly connected to your question, but don't make the mistake of calling the Runnable objects run() method, instead of the Thread objects start() method. A mistake that is quite common :S. – m2o Jun 20 '11 at 12:02
  • @Oltarus, for non-windows based systems, find a java runtime process id and use `kill -9 pid` where `pid` is the java runtime process id. Happy? :P – Buhake Sindi Jun 20 '11 at 12:05
  • @The Elite Gentleman Delighted! – SteeveDroz Jun 20 '11 at 12:06
  • @The Elite Gentleman, windows command is not as famous as `kill -9` but it does exist: `taskkill /pid` or in your case `taskkill /IM javaw.exe` – bestsss Jun 20 '11 at 14:16

6 Answers6

49

First of all, you are not starting any thread here! You should create a new thread and pass your confusingly named thread1 Runnable to it:

thread1 t1 = new thread1();
final Thread thread = new Thread(t1);
thread.start();

Now, when you really have a thread, there is a built in feature to interrupt running threads, called... interrupt():

thread.interrupt();

However, setting this flag alone does nothing, you have to handle this in your running thread:

while(!Thread.currentThread().isInterrupted()){
    try{        
        Thread.sleep(10);
    }
    catch(InterruptedException e){
        Thread.currentThread().interrupt();
        break; //optional, since the while loop conditional should detect the interrupted state
    }
    catch(Exception e){
        e.printStackTrace();
    }

Two things to note: while loop will now end when thread isInterrupted(). But if the thread is interrupted during sleep, JVM is so kind it will inform you about by throwing InterruptedException out of sleep(). Catch it and break your loop. That's it!


As for other suggestions:

Deprecated. This method is inherently unsafe[...]

  • Adding your own flag and keeping an eye on it is fine (just remember to use AtomicBoolean or volatile!), but why bother if JDK already provides you a built-in flag like this? The added benefit is interrupting sleeps, making thread interruption more responsive.
Tim Bender
  • 20,112
  • 2
  • 49
  • 58
Tomasz Nurkiewicz
  • 334,321
  • 69
  • 703
  • 674
  • most likely you want to check it `Thread.interrupted()` (the method clears the flag) – bestsss Jun 20 '11 at 12:46
  • +1, but I added re-setting the interrupt flag. Generally it is a good idea to continue with best practices. Clearing the interrupt flag (as @bestsss has recommended) is generally not a good idea since the use context is not known. What if the OP is trying to write a util or some lib/client code? You wouldn't want to have to use some API which naughtily hid an interrupt request from the main program, would you? – Tim Bender Jun 20 '11 at 21:41
  • @Tim, the code didn't set the flag, so it's `Thread.interrupt()` but... best practice is to throw something. While leaving the flag does help, it's a lot better to throw something so the code flow transfers quickly out of the scope, instead of polling the flag in each few statements. I do not agree that constantly polling the flag is the best option. Library code usually do that, i.e. `Thread.interrupted() + throw InterruptedException` you can see it in java.util.concurrent.locks – bestsss Jun 20 '11 at 22:23
  • @bestsss ... right, it throws an InterruptedException so that you don't have to check the flag at the return of every blocking operation. However, in handling an InterruptedException, the proper thing for an API to do is to either re-throw it, or re-set the interrupt flag. You can read this IBM white paper http://www.ibm.com/developerworks/java/library/j-jtp05236/index.html or read Brian Goetz "Java Concurrency in Practice" for more detail. P.S. the method `interrupt()` on `Thread` is not `static` – Tim Bender Jun 20 '11 at 22:42
5

The proper way to stop a thread is to interrupt it (stop() is deprecated and may have nasty side effects):

t1.interrupt()

This will cause an InterruptedException to be thrown by methods like Thread.sleep() or Object.wait().

Then just add a catch block for this exception and simply break out of the while loop.

EDIT: I now realised that your infinite loop is running within the main thread, there's no thread created by your code, it's just run()ning a Runnable. You need to call Thread.start() at some point to spawn a new thread.

Costi Ciudatu
  • 37,042
  • 7
  • 56
  • 92
4

Move the catch interrupt to outside the loop. This doesn't require any more lines of code, it just handles interrupts correctly i.e. the action is interrupted.

public void run() {
    try{        
        while(true) {
            Thread.sleep(10);
        }
    } catch(InterruptedException e){
        System.out.println("Thread interrupted"));
    }
}
Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
2

Create a field boolean keepGoing that you set to true before starting your thread and replace while (true) with while (keepGoing). At some point, you decide where, simply change the value of keepGoing to false and it will exit the loop.

SteeveDroz
  • 6,006
  • 6
  • 33
  • 65
  • @Tomasz Nurkiewicz Fixed, replaced it by `keepGoing`! – SteeveDroz Jun 20 '11 at 12:15
  • 1
    -1 because this in essence duplicates the functionality of the interrupt flag but in a very non-standard way without any mention/consideration for thread visibility. – Tim Bender Jun 20 '11 at 21:36
2

The only way to stop an arbitrary thread is by interrupting it. Keep a reference to it then call the interrupt method.

Bohemian
  • 412,405
  • 93
  • 575
  • 722
1

I recommend using Thread.interrupt() (as mentioned by @Bohemian). It has a couple of advantages over using ad-hoc flags:

  • You don't need to create and use an application-specific API to do this. (And interrupts are guaranteed thread-safe ...)

  • Thread.interrupt() will interrupt threads that are blocked in a wait() or a join, or possibly1 some blocking I/O calls.

However, it is not a magic bullet. If the thread you are trying to stop is executing regular code, it needs to periodically check its interrupted() flag, or it won't no to stop. This leaves us in the same as boat as we are in with an ad-hoc flag mechanism. The thread has to cooperate, or it can't be (safely) stopped.

1 - This is a murky area. On the one hand, there is an InterruptedIOException whose javadoc says "Signals that an I/O operation has been interrupted". On the other hand, the exception is not explicitly mentioned in the javadocs for the various java.io stream classes.


It is true that some 3rd-party code may not deal with the interrupted flag properly, and interrupts may get "eaten" as a result. But you can check for that if you have source code. And the situation is not a lot different to the 3rd-party code not paying attention to your ad-hoc flag mechanism.


I would NOT recommend using Thread.stop(). It is fundamentally flakey. Some people claim that it works for them, but IMO they are either dealing with a special case that works ... or they are being lucky.

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
  • @Stephen, a lot of code (call it 3rd party) tends to eat the flag (or doesn't reset it). btw, thread suspend/stop and checking the stack (crawling it) for dangerous stop locations seems to do the trick for me. – bestsss Jun 20 '11 at 12:42
  • @bestsss - I would not recommend doing that! It requires deep understanding of your code-base (to know where the dangerous places are) and is fragile. (The next guy touching the code may not be so familiar with it ... ) – Stephen C Jun 20 '11 at 12:49
  • @Stephen, the real fragile code is the one of the likes: lock.lock(); try{}finally( **lock.unlock()**} getting ThreadDeath in the finally block totally blows indeed. any stacks location in `java.util.concurrent.locks.` are truly bad, OTOH most of the java.net is good to stop. I'd never recommend anyone trying on their own since it's also (I suppose) JVM dependent and so on. It's somehow a pity Java doesn't have C# thread stop semantics. – bestsss Jun 20 '11 at 12:53
  • @bestsss - C# `Thread.Abort()` apparently has the same problems; see http://stackoverflow.com/questions/1327102/how-to-kill-a-thread-instantly-in-c – Stephen C Jun 20 '11 at 13:09
  • @Stephen, but it does execute the finally blocks and it cannot be caught and eaten (i.e. after catch it's always re-thrown). It's no panacea but java version mauls the j.u.c.Locks read bad, that's the single gripe I have, if you happen to fall upon some truly global lock, you're screwed. Of course, I do not stop theads via stop() for fun but sometimes there is no other way (not my choice). I don't like restarting a process, the servers must run for months, until the core code needs some fix. – bestsss Jun 20 '11 at 13:57
  • The requirement that processes / servers **must** run for months sounds to me like self-inflicted pain. – Stephen C Jun 20 '11 at 23:20
  • @Stephen, I am not talking of web-servers. Leaks are unacceptable to me. If you have no leaks, there is no need to restart. Imagine you have multicore MMO game server w/ shared memory; would you like to restart it? Not exactly my case but not that different. – bestsss Jun 21 '11 at 06:18
  • @bestsss - there is qualitatively no difference. If you've got an application with threads that need killing, then you've got a poorly designed application and/or bugs. If you've got an application with no provision for fail-over and/or occasional restarts, then you've got a poorly designed application. A system that requires constant uptime for one/each instance of a component is poorly designed. The best solution is to fix the design ... not to take "heroic" measures to avoid restarts. – Stephen C Jun 21 '11 at 06:25
  • @Stephen, threads "need killing" due to some unexpected bug/condition, it's not the norm but it happens. Being able to fix a bug w/o losing few thousands sockets (and associated client states) is a feat to me. It takes less than a second to redeploy some module. I gave a game as example, no way it's possible to shutdown a whole server w/o significant adverse effects on the players, regardless if there would be any server to carry the load. Never I told clients should not be able to recover if a server goes puff but that's no plan for proper experience for the clients. – bestsss Jun 21 '11 at 22:31