Porting code from using timers to scheduledexecutorservice

Question

I am trying to port code from using java timers to using scheduledexecutorservice

I have the following use case

class A {

    public boolean execute() {
         try {
              Timer t = new Timer();
              t.schedule (new ATimerTask(), period, delay);
         } catch (Exception e) {
              return false;
         }
    }

}


class B {

    public boolean execute() {
         try {
              Timer t = new Timer();
              t.schedule (new BTimerTask(), period, delay);
         } catch (Exception e) {
              return false;
         }
    }

}

Should I just replace Timer instances in class A and class B with ScheduledExecutorService and make the ATimerTask and BTimerTask class to a Runnable class , for e.g

class B {

    public boolean execute() {
         try {
              final ScheduledExecutorService scheduler = 
   Executors.newScheduledThreadPool(1);

              scheduler.scheduleWithFixedDelay (new BRunnnableTask(), period, delay);
         } catch (Exception e) {
              return false;
         }
    }

}

Is this correct.

EDIT: One of the primary motivation of porting is since runtime exceptions thrown in TimerTask kill that one thread and it cannot be scheduled further. I want to avoid the case so that ieven if I have runtime exception the thread should keep on executing and not halt.

Answer 1

It looks ok. Depending on what you're doing, you may want to keep executor service around as a member so you can use it again. Also, you can get a ScheduledFuture back from the scheduleXX() methods. This is useful because you can call get() on it to pull any exceptions that occur in the timed thread back to your control thread for handling.

Answer 2

NOTE: The way you did this will leak threads!

If your class B will be kept around and each instance will eventually be closed or shut down or released, I would do it like this:

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

  public boolean execute() {
    try {
      scheduler.scheduleWithFixedDelay(new BRunnnableTask(), period, delay);
      return true;
    } catch (Exception e) {
      return false;
    }
  }

  public void close() {
    scheduler.shutdownNow();
  }
}

If you will not do this kind of cleanup on each instance, then I would instead do this:

class B {
  static final ScheduledExecutorService SCHEDULER = Executors.newCachedThreadPool();

  public boolean execute() {
    try {
      SCHEDULER.scheduleWithFixedDelay(new BRunnnableTask(), period, delay);
      return true;
    } catch (Exception e) {
      return false;
    }
  }
}

Each ExecutorService you allocate in your code allocates a single Thread. If you make many instances of your class B then each instance will be allocated a Thread. If these don't get garbage collected quickly, then you can end up with many thousands of threads allocated (but not used, just allocated) and you can crash your whole server, starving every process on the machine, not just your own JVM. I've seen it happen on Windows and I expect it can happen on other OS's as well.

A static Cached thread pool is very often a safe solution when you don't intend to use lifecycle methods on the individual object instances, as you'll only keep as many threads as are actually running and not one for each instance you create that is not yet garbage collected.

Answer 3

There is currently no good way to handle repeating tasks in the Executors framework.

It really wasn't designed with this use case in mind, and there is no realistic way to avoid swallowing exceptions.

If you really must use it for repeating tasks, each scheduling should look something like this:

scheduler.scheduleWithFixedDelay(new Runnable() {
  public void run() {
     try {
       .. your normal code here...
     } catch (Throwable t) {
       // handle exceptions there
     }
  }
}, period, delay);