From 55b4bfa1fe25d1e91c73a07ed883d277ae39e59f Mon Sep 17 00:00:00 2001
From: Ethan Buchman <ethan@coinculture.info>
Date: Tue, 10 Jan 2017 20:37:32 -0500
Subject: [PATCH] consensus: let time.Timer handle non-positive durations

---
 consensus/replay.go          |  4 +--
 consensus/state.go           |  3 ---
 consensus/test_data/build.sh |  3 +++
 consensus/ticker.go          | 47 +++++++++++++++++++++++-------------
 4 files changed, 35 insertions(+), 22 deletions(-)

diff --git a/consensus/replay.go b/consensus/replay.go
index 124d3b5f..8793c42f 100644
--- a/consensus/replay.go
+++ b/consensus/replay.go
@@ -246,9 +246,9 @@ func (cs *ConsensusState) startForReplay() {
 	// don't want to start full cs
 	cs.BaseService.OnStart()
 
+	log.Warn("Replay commands are disabled until someone updates them and writes tests")
+	/* TODO:!
 	// since we replay tocks we just ignore ticks
-	// TODO:!
-	/*
 		go func() {
 			for {
 				select {
diff --git a/consensus/state.go b/consensus/state.go
index 876a3cb7..594cebd6 100644
--- a/consensus/state.go
+++ b/consensus/state.go
@@ -488,9 +488,6 @@ func (cs *ConsensusState) updateRoundStep(round int, step RoundStepType) {
 func (cs *ConsensusState) scheduleRound0(rs *RoundState) {
 	//log.Info("scheduleRound0", "now", time.Now(), "startTime", cs.StartTime)
 	sleepDuration := rs.StartTime.Sub(time.Now())
-	if sleepDuration < time.Duration(0) {
-		sleepDuration = time.Duration(0)
-	}
 	cs.scheduleTimeout(sleepDuration, rs.Height, 0, RoundStepNewHeight)
 }
 
diff --git a/consensus/test_data/build.sh b/consensus/test_data/build.sh
index de0e264b..970eb783 100644
--- a/consensus/test_data/build.sh
+++ b/consensus/test_data/build.sh
@@ -22,6 +22,9 @@ tendermint node --proxy_app=dummy &> /dev/null &
 sleep 5
 killall tendermint
 
+# /q would print up to and including the match, then quit. 
+# /Q doesn't include the match. 
+# http://unix.stackexchange.com/questions/11305/grep-show-all-the-file-up-to-the-match
 sed '/HEIGHT: 2/Q' ~/.tendermint/data/cs.wal/wal  > consensus/test_data/empty_block.cswal
 
 reset
diff --git a/consensus/ticker.go b/consensus/ticker.go
index 1550ca16..06a8f7d2 100644
--- a/consensus/ticker.go
+++ b/consensus/ticker.go
@@ -10,6 +10,9 @@ var (
 	tickTockBufferSize = 10
 )
 
+// TimeoutTicker is a timer that schedules timeouts
+// conditional on the height/round/step in the timeoutInfo.
+// The timeoutInfo.Duration may be non-positive.
 type TimeoutTicker interface {
 	Start() (bool, error)
 	Stop() bool
@@ -17,6 +20,11 @@ type TimeoutTicker interface {
 	ScheduleTimeout(ti timeoutInfo) // reset the timer
 }
 
+// timeoutTicker wraps time.Timer,
+// scheduling timeouts only for greater height/round/step
+// than what it's already seen.
+// Timeouts are scheduled along the tickChan,
+// and fired on the tockChan.
 type timeoutTicker struct {
 	BaseService
 
@@ -31,9 +39,7 @@ func NewTimeoutTicker() TimeoutTicker {
 		tickChan: make(chan timeoutInfo, tickTockBufferSize),
 		tockChan: make(chan timeoutInfo, tickTockBufferSize),
 	}
-	if !tt.timer.Stop() {
-		<-tt.timer.C
-	}
+	tt.stopTimer() // don't want to fire until the first scheduled timeout
 	tt.BaseService = *NewBaseService(log, "TimeoutTicker", tt)
 	return tt
 }
@@ -48,6 +54,7 @@ func (t *timeoutTicker) OnStart() error {
 
 func (t *timeoutTicker) OnStop() {
 	t.BaseService.OnStop()
+	t.stopTimer()
 }
 
 func (t *timeoutTicker) Chan() <-chan timeoutInfo {
@@ -60,6 +67,20 @@ func (t *timeoutTicker) ScheduleTimeout(ti timeoutInfo) {
 	t.tickChan <- ti
 }
 
+//-------------------------------------------------------------
+
+// stop the timer and drain if necessary
+func (t *timeoutTicker) stopTimer() {
+	// Stop() returns false if it was already fired or was stopped
+	if !t.timer.Stop() {
+		select {
+		case <-t.timer.C:
+		default:
+			log.Debug("Timer already stopped")
+		}
+	}
+}
+
 // send on tickChan to start a new timer.
 // timers are interupted and replaced by new ticks from later steps
 // timeouts of 0 on the tickChan will be immediately relayed to the tockChan
@@ -84,22 +105,14 @@ func (t *timeoutTicker) timeoutRoutine() {
 				}
 			}
 
+			// stop the last timer
+			t.stopTimer()
+
+			// update timeoutInfo and reset timer
+			// NOTE time.Timer allows duration to be non-positive
 			ti = newti
-
-			// if the newti has duration == 0, we relay to the tockChan immediately (no timeout)
-			if ti.Duration == time.Duration(0) {
-				go func(toi timeoutInfo) { t.tockChan <- toi }(ti)
-				continue
-			}
-
-			log.Debug("Scheduling timeout", "dur", ti.Duration, "height", ti.Height, "round", ti.Round, "step", ti.Step)
-			if !t.timer.Stop() {
-				select {
-				case <-t.timer.C:
-				default:
-				}
-			}
 			t.timer.Reset(ti.Duration)
+			log.Debug("Scheduled timeout", "dur", ti.Duration, "height", ti.Height, "round", ti.Round, "step", ti.Step)
 		case <-t.timer.C:
 			log.Info("Timed out", "dur", ti.Duration, "height", ti.Height, "round", ti.Round, "step", ti.Step)
 			// go routine here gaurantees timeoutRoutine doesn't block.