views:

291

answers:

5

I wrote a thread, it is taking too much time to execute and it seems it is not completely done. I want to stop the thread gracefully. Any help ?

+1  A: 

Make a volatile boolean stop somewhere. Then in the code that runs in the thread, regularly do

 if (stop) // end gracefully by breaking out of loop or whatever

To stop the thread, set stop to true.

I think you must do it manually this way. After all, only the code running in the thread has any idea what is and isn't graceful.

Bart van Heukelom
Note that you either need to use locking or make the field volatile to make sure the reading thread sees changes from the writing thread.
Jon Skeet
@Jon Skeet: Yes, that would be better. Out of curiosity though, how long would the stopping approximately be delayed if the field is not volatile?
Bart van Heukelom
@Bart: In pratice, probably not long at all. In theory, forever.
Jon Skeet
@Jon Skeet: Really? Wouldn't that mean that in a theoretically correct multithreaded program many fields need to be volatile? I'll need to read up on this some more when I get the chance.
Bart van Heukelom
@Bart: Normally if a multi-threaded program needs to share mutable state, it will use locking instead of volatility. Sharing immutable state is much simpler, of course.
Jon Skeet
@Bart: using a non-volatile boolean, the following loopwhile(!stopped){ doStuff();} can be optimized to if(!stopped)return;while(true){doStuff();} by the compiler
mpm
+12  A: 

The good way to do it is to have the run() of the Thread guarded by a boolean variable and set it to from outside when you want to stop it, something like:

class MyThread extends Thread
{
  volatile boolean finished = false;

  public void stopMe()
  {
    finished = true;
  }

  public void run()
  {
    while (!finished)
    {
      //do dirty work
    }
  }
}

Once upon a time a stop() method existed but as the documentation states

This method is inherently unsafe. Stopping a thread with Thread.stop causes it to unlock all of the monitors that it has locked (as a natural consequence of the unchecked ThreadDeath exception propagating up the stack). If any of the objects previously protected by these monitors were in an inconsistent state, the damaged objects become visible to other threads, potentially resulting in arbitrary behavior.

That's why you should have a guard..

Jack
See Jon Skeet's comment to Bart's answer - it applies to yours as well.
Péter Török
If "finished" change while you are doing your "dirty work" thread won't end, then you have to do a periodic check, for example with a sleep(x) in the loop, something that is not recomended and gives you a bad performance, so is really this a good aproach?
Hernán Eche
we are talking about quitting a thread gracefully. Forcing a thread to quit in the middle of its dirty work is not a graceful way to do it. You should always wait for next iteration unless you have got a specific reason to interrupt it..
Jack
you can use wait and notify to avoid expressly loop checking
Hernán Eche
+2  A: 

You should not kill Thread from other one. It's considered as fairly bad habit. However, there are many ways. You can use return statement from thread's run method. Or you can check if thread has already been interrupted and then it will cancel it's work. F.e. :

while (!isInterrupted()) { 
  // doStuff
}
Xorty
The run method is void, it can't return anything.
Bart van Heukelom
you can use empty return statement with void return type, method will end
Xorty
this is similar to the first answer, except you're using return instead of simply letting the run() method exit.And yes, you can just use return; and that will exit from a void method
Richard
Well the most voted answer wasn't the first afaik
Xorty
Ah, I misunderstood. I thought you wanted to return a value from the run and do something (whatever) with that. Of course, using return to quit the method is perfectly possible.
Bart van Heukelom
A: 

You need to send a stop-message to the Thread and the Thread itself needs to take action if the message has been received. This is pretty easy, if the long-running action is inside loop:

public class StoppableThread extends Thread {

   private volatile boolean stop = false;

   public void stopGracefully() {
     stop = true;
   }

   public void run() {
     boolean finished = false;
     while (!stop && !finished) {
       // long running action - finished will be true once work is done
     }
   }
}
Andreas_D
As Jon Skeet remarked on my answer, stop should be volatile
Bart van Heukelom
I edited my answer but will go and find out, why it applies here - in your answer, I thought, it had to be volatile just because you didn't mention a method and it looked like, the `stop` field had to be accessed directly.
Andreas_D
I've wondered about the volatile keyword before and (without trying to advertise my own question :) ), here's a few answers on it:http://stackoverflow.com/questions/106591/do-you-ever-use-the-volatile-keyword-in-java
Richard
+1  A: 

The bad part about using a flag to stop your thread is that if the thread is waiting or sleeping then you have to wait for it to finish waiting/sleeping. If you call the interrupt method on the thread then that will cause the wait or sleep call to be exited with an InterruptedException. You can write the thread's run method so that the InterruptedException is caught outside whatever looping logic the thread is doing.

(BTW calling interrupt() also sets an interrupted property that you can use as a flag to check whether to quit.)

Nathan Hughes