Reputation: 13946
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?
Upvotes: 3
Views: 1435
Reputation: 27183
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);
}
}
}
Upvotes: 0
Reputation: 7989
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);
}
}}
Upvotes: 0
Reputation: 691635
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.
Upvotes: 3