0

I use ScheduledExecutorService to start a timer that runs periodically, but this timer can't be canceled after I calling the cancel():

import java.util.concurrent.*;

public class Monitor {
    private static ScheduledFuture<?> timerCtrl;
    private final ScheduledExecutorService scheduExec = Executors.newScheduledThreadPool(1, StatmonitorThreadFactory.getInstance());

    private void startTimer() {
        timerCtrl = scheduExec.scheduleAtFixedRate(new MonitorTimer(), 5, 5, TimeUnit.SECONDS);
    }

    public boolean cancelMonitorTimer() {
        if (timerCtrl != null) {
            timerCtrl.cancel(false); //both timerCtrl.isDone() and timerCtrl.isCancelled() return true
            LOG.error("{} {}", timerCtrl.isDone(), timerCtrl.isCancelled());
            if (!timerCtrl.isCancelled()) {
                LOG.error("timerCtrl cancel failed!");
                return false;
            }
        }
        return true;
    }

    private class MonitorTimer implements Runnable {
        @Override
        public void run() {
            doPeriodicMonitor(); //call another function
        }
    }
}

At first, I call startTimer() to start my timer. After a while, I call cancelMonitorTimer to cancel and stop this timer and the function return true but timer still runs, doPeriodicMonitor is called every 5 seconds which is a period set by myself in startTimer.

vinllen
  • 1,369
  • 2
  • 18
  • 36
  • This would be expected behavior if the monitoring task takes longer than 5 seconds and the thread pool is exhausted. You should add logging to record the latency of `doPeriodicMonitor` and then ensure the scheduled rate is slower. You should also try it without the thread pool to rule out a bug there. – Gene Mar 02 '17 at 06:44
  • @Gene In `doPeriodicMonitor()`, I start another two threads to do some work, si it illegal? – vinllen Mar 02 '17 at 07:12

2 Answers2

0

You need to set true to timeCrtl.cancel(true) instead of timerCtrl.cancel(false) in your cancelMonitorTimer() method. This function further returns a boolean value true if the the task is cancelled successfully but if it returns false then it means it has already been completed.

Nikhil Mahajan
  • 523
  • 5
  • 17
  • not works, `mayInterruptIfRunning - true if the thread executing this task should be interrupted; otherwise, in-progress tasks are allowed to complete`, the parameter means to interrupt the running thread. In this case, my function `doPeriodicMonitor ` is called every 5 seconds – vinllen Mar 02 '17 at 06:33
0

I have found the reason that I should not use qualifier static in defining timerCtrl. But I still have no idea why can't. In my understanding, static value is per-class-loader but not per-thread, so that this variable will be shared between different threads.
In addition, class Monitor is a base class which was inherited by another class called MonitorSon, and the operation is called in the son class.

Community
  • 1
  • 1
vinllen
  • 1,369
  • 2
  • 18
  • 36
  • The `static` field is per-classloader. As per the answer you quoted, there are synchronization problems when multiple threads are updating the same `static` field in terms of race conditions and memory sharing but they are still the same field. If thread1 assigns `timeCtrl` then if it is static thread2 will overwrite thread1's value. – Gray May 15 '17 at 12:46