0

I would like to restart my timer when I receive a KeepAlive, the problem is that sometimes it doesn't restart, instead of that it creates a new one, so, finally the timer reaches the limit:

public class KeepAliveTimer {


long macAddress;
Timer timer;
String ip;  


public KeepAliveTimer(long mac, String ipAddress){
    this.macAddress = mac;
    this.ip = ipAddress;

    timer = new Timer();

    TimerTask timerTask = new TimerTask() {

        @Override
        public void run() {

            timerFinished();

        }
    };

    timer.schedule(timerTask, 10*60*1000);
}


public void update() {


    TimerTask timerTask = new TimerTask() {

        @Override
        public void run() {
            timerFinished();

        }
    };
    timer.cancel();
    timer.purge();
    timer = new Timer();
    timer.schedule(timerTask, 10*60*1000);
}

public void timerFinished() {

    //tasks
}

}

The object KeepAliveTimer is created when received the first keepAlive and updated by following ones

user1256477
  • 10,763
  • 7
  • 38
  • 62

1 Answers1

1

You should not cancel the whole timer, but only the TimerTask. This is how I'd write your code:

public class KeepAliveTimer {
  final Timer timer = new Timer();
  final long macAddress;
  final String ip;
  volatile TimerTask timerTask;

  public KeepAliveTimer(long mac, String ipAddress) {
    this.macAddress = mac;
    this.ip = ipAddress;
  }
  public void update() {
    if (timerTask != null) timerTask.cancel();
    timer.schedule(timerTask(), 10 * 60 * 1000);
  }
  private TimerTask timerTask() {
    return timerTask = new TimerTask() {
      @Override public void run() { timerFinished(); }
    };
  }
  public void timerFinished() {
    // tasks
  }
}

Note that I don't duplicate the scheduling inside the constructor. You should initialize with

new KeepAliveTimer(mac, ip).update();
Marko Topolnik
  • 195,646
  • 29
  • 319
  • 436