Properly writing task that ExecutorService.shutdownNow() can stop?

Question

I have a processing loop of the form

while (true) {
    doWork();
    Thread.sleep(SLEEP_INTERVAL);
}

I want to make a Runnable out of this that can play well with ExecutorService and which will exit when ExecutorService.shutdownNow() is called.

I'm looking to write it this way:

public WorkerTask implements Runnable
{
    @Override
    public void run() {
        while (!Thread.currentThread().isInterrupted()) {
            doWork();
            try {
                Thread.sleep(SLEEP_INTERVAL);
            }
            catch (InterruptedException e) {
                Thread.currentThread().interrupt();
            }
        }
    }
}

Simple testing shows it at least appearing to work in that the task gets interrupted and will exit and the ExecutorService will shut down, and appears to do so whether the interrupt arrives while doWork() is processing or during the sleep. (By varying how much work doWork() does and how big SLEEP_INTERVAL is I can pretty much control where the interrupt happens).

But when I google I see examples using Thread.interrupted() as well as Thread.currentThread().isInterrupted(). I understand that the former clears the interrupted flag while the latter leaves it, but is there any other difference I need to care about?

I also see versions where the result of Thread.currentThread().isInterrupted() or Thread.interrupted() is stored in a volatile variable and that variable is used as the while loop test condition. Is that just a style or is there a need to do that? In what I've written do I have to worry that somehow something can clear the interrupt flag between when it is set (whether by being received when the thread is live, or by my catching InterruptedException and reasserting the flag) and when Thread.currentThread().isInterrupted() is called in the loop test?

Answer 1

Your code looks fine to me. Introducing an additional volatile variable would be unnecessary complexity: the interrupt status does the job.

The recommended way, in Java Concurrency in Practice, to deal with interrupts in tasks is to either throw an InterruptedException (this is doable if the task is a Callable and not a Runnable), or to make sure the interrupt status is set and to exit from the task ASAP. Your code does that well.

Answer 2

Could you take a look at ScheduledExecutorService if it matches your requirements:

 class BeeperControl {
    private final ScheduledExecutorService scheduler =
      Executors.newScheduledThreadPool(1);

    public void beepForAnHour() {
      final Runnable beeper = new Runnable() {
        public void run() { System.out.println("beep"); }
      };
      final ScheduledFuture<?> beeperHandle =
        scheduler.scheduleAtFixedRate(beeper, 10, 10, SECONDS);
      scheduler.schedule(new Runnable() {
        public void run() { beeperHandle.cancel(true); }
      }, 60 * 60, SECONDS);
    }
  }}

Answer 3

Basically you should take advantage of java.util.concurrent libraries here .You should submit your task via ExecutorService.submit()and then call blocking methods like Future.get() , then you can be sure that those methods will respond to interruption as soon as possible by throwing an ExecutionException() .You probably should get rid of that Thread.sleep() since it is doing nothing . You want to sniff an interrupt as quickly as possible .You possibly also want to wait for a timeout in case your task is doing something inifinitely . So if the task terminates with a TimeOutException , the task is cancelled via its Future. I call cancel() unconditionally since cancelling a completed task has no effect. In that case you can do some thing like :

 public static void main(String[] args) {
    WorkerTask runnable;
    TimeUnit unit;
    Future<?>   task = executor.submit(workerTask);
    try{
      task.get(timeout,unit);
    } catch(TimeoutException e){

    }catch(ExecutionException e){
        throw e.getCause();  
    } finally{
      //Harmless if the task already completed
      task.cancel(true);
    }
  }
}