EJB3 TimerService spec needs fixing

We've gotten many complaints about our scoring bug: people just leave the game (which they are presumably losing) time-out and do NOT lose any points ... how can this be?

It shouldn't happen and the issue has been fixed.

The error was in a little code of "silly logic" which would create clearner timers but then also kill them before they got a chance to run, the issue was thread related (as usual). How I fixed it?
(Read on if you care about the technical details of the bug in our Java EE system)

Using TimerService we have set up two timers in the EJB3 session beans that manage game creation (matching opponents etc.) and game play (including scoring). These timers execute the @Timeout methods every few seconds to make sure people get their points and there are no stale old games sitting around:


@Stateless
public class GameManagerBean implements GameManagerRemote, GameManagerLocal {

   @Timeout
   private void cleanUnmatchedGames(Timer t) {
     ... clean unmatched abandoned games
   }

   public void startGameCleanerTimer() {
     this.stopGameCleanerTimer();
     this.timerService.createTimer(PERIOD_MS, PERIOD_MS, GAME_TIMER_ID);
   }

   public void stopGameCleanerTimer() {
     for (Iterator it = this.timerService.getTimers().iterator(); it.hasNext();) {
       Timer timer = (Timer) it.next();
       if (timer.getInfo().equals(GAME_TIMER_ID)) {
         timer.cancel();
       }
     }
   }

}

@Stateless
public class PlayManagerBean implements PlayManagerRemote, PlayManagerLocal {

   @Timeout
   private void cleanTimedOutGames(Timer t) {
     ... clean games where one player timed-out or left
   }

   public void startPlayCleanerTimer() {
     this.stopGameCleanerTimer();
     this.timerService.createTimer(PERIOD_MS, PERIOD_MS, PLAY_TIMER_ID);
   }

   public void stopPlayCleanerTimer() {
     for (Iterator it = this.timerService.getTimers().iterator(); it.hasNext();) {
       Timer timer = (Timer) it.next();
       if (timer.getInfo().equals(PLAY_TIMER_ID)) {
         timer.cancel();
       }
     }
   }

}


To be certain that the above @Timeout methods are called by a single timer at regular intervals we have to make sure that one and only one timer of each kind is alive at any one time. Timers are persistent (survive server crash or shutdown) which can be nice but it can also be a pain since it means that if you start a timer every-time your server starts you'll be accumulating them. To make sure that we create timers once per server startup and delete them on server shutdown I put the code into servlet life-cycle methods.


public class MobileServiceServlet extends HttpServlet {
   public void init() throws ServletException {
     this.gameManagerBean.startGameCleanerTimer();
     this.playManagerBean.startPlayCleanerTimer();
   }

   public void destroy() {
     this.gameManagerBean.stopGameCleanerTimer();
     this.playManagerBean.stopPlayCleanerTimer();
   }
}


You may have noticed that to ensure that only one timer of a kind is alive at a time the startGameCleanerTimer() and startPlayCleanerTimer()methods also delete any old timers which may be around due to server crash. Just to be on a safe side.

All these code gymnastics just to make sure that we aren't starting multiple timers, I hope that the next Java EE spec will include some "on deployment" related features so that this is no longer necessary.