using ScheduledExecutorService to start and stop timer
From my readings, it seems that ScheduledExecutorService is the right way to start and stop timers in Java.
I need to port some code that starts and stops a timer. This is not a periodic timer. This code, stops the timer before starting it. So, effectively every start is really a restart(). I am looking for the right way to do this using the ScheduledExecutorService. Here is what I came up with. Looking for comments and insight on things I am missing:
ScheduledExecutorService _Timer = Executors.newScheduledThreadPool(1);
ScheduledFuture<?> _TimerFuture = null;
private boolean startTimer() {
try {
if (_TimerFuture != null) {
//cancel execution of the future task (TimerPopTask())
//If task is already running, do not interrupt it.
_TimerFuture.cancel(false);
}
_TimerFuture = _Timer.schedule(new TimerPopTask(),
TIMER_IN_SECONDS,
TimeUnit.SECONDS);
return true;
} catch (Exception e) {
return false;
}
}
private boolean stopTimer() {
try {
if (_TimerFuture != null) {
//cancel execution of the future task (TimerPopTask())
//If task is already running, interrupt it here.
_TimerFuture.cancel(true);
}
开发者_JAVA技巧 return true;
} catch (Exception e) {
return false;
}
}
private class TimerPopTask implements Runnable {
public void run () {
TimerPopped();
}
}
public void TimerPopped () {
//Do Something
}
tia, rouble
This looks like a problem:
private boolean startTimer() {
// ......
if (_TimerFuture != null) {
_TimerFuture.cancel(false);
}
_TimerFuture = _Timer.schedule(new TimerPopTask(),
TIMER_IN_SECONDS,
TimeUnit.SECONDS);
// ......
}
Since you're passing a false to cancel, the old _TimerFuture
may not get cancelled if the task is already running. A new one gets created anyway (but it won't run concurrently because your ExecutorService
has a fixed thread pool size of 1). In any case, that doesn't sound like your desired behavior of restarting a timer when startTimer() is called.
I would rearchitect a bit. I would make the TimerPopTask
instance be the thing you "cancel", and I would leave the ScheduledFutures
alone once they are created:
private class TimerPopTask implements Runnable {
//volatile for thread-safety
private volatile boolean isActive = true;
public void run () {
if (isActive){
TimerPopped();
}
}
public void deactivate(){
isActive = false;
}
}
then I would retain the instance of TimerPopTask
rather than the instance of ScheduledFuture
and rearrange startTimer method thusly:
private TimerPopTask timerPopTask;
private boolean startTimer() {
try {
if (timerPopTask != null) {
timerPopTask.deactivate();
}
timerPopTask = new TimerPopTask();
_Timer.schedule(timerPopTask,
TIMER_IN_SECONDS,
TimeUnit.SECONDS);
return true;
} catch (Exception e) {
return false;
}
}
(Similar modification to stopTimer() method.)
You may want to crank up the number of threads if you truly anticipate needing to 'restart' the timer before the current timer expires:
private ScheduledExecutorService _Timer = Executors.newScheduledThreadPool(5);
You may want to go with a hybrid approach, keeping references to both the current TimerPopTask as I described and also to the current ScheduledFuture and make the best effort to cancel it and free up the thread if possible, understanding that it's not guaranteed to cancel.
(Note: this all assumes startTimer() and stopTimer() method calls are confined to a single main thread, and only the TimerPopTask
instances are shared between threads. Otherwise you'll need additional safeguards.)
精彩评论