diff --git a/common/repeat_timer.go b/common/repeat_timer.go index 734c2d32..b3eb107d 100644 --- a/common/repeat_timer.go +++ b/common/repeat_timer.go @@ -1,7 +1,6 @@ package common import ( - "fmt" "time" ) @@ -17,7 +16,7 @@ type RepeatTimer struct { input chan repeatCommand dur time.Duration - timer *time.Timer + ticker *time.Ticker stopped bool } @@ -36,8 +35,8 @@ func NewRepeatTimer(name string, dur time.Duration) *RepeatTimer { output: c, input: make(chan repeatCommand), - timer: time.NewTimer(dur), - dur: dur, + dur: dur, + ticker: time.NewTicker(dur), } go t.run() return t @@ -51,6 +50,7 @@ func (t *RepeatTimer) Reset() { // For ease of .Stop()'ing services before .Start()'ing them, // we ignore .Stop()'s on nil RepeatTimers. func (t *RepeatTimer) Stop() bool { + // use t.stopped to gracefully handle many Stop() without blocking if t == nil || t.stopped { return false } @@ -67,39 +67,33 @@ func (t *RepeatTimer) run() { // stop goroutine if the input says so // don't close channels, as closed channels mess up select reads done = t.processInput(cmd) - case <-t.timer.C: - // send if not blocked, then start the next tick + case <-t.ticker.C: t.trySend() - t.timer.Reset(t.dur) } } - fmt.Println("end run") } // trySend performs non-blocking send on t.Ch func (t *RepeatTimer) trySend() { - // TODO: this was blocking in previous version (t.Ch <- t_) + // NOTE: this was blocking in previous version (t.Ch <- t_) // should I use that behavior unstead of unblocking as per throttle? - - // select { - // case t.output <- time.Now(): - // default: - // } - - t.output <- time.Now() + // probably not: https://golang.org/src/time/sleep.go#L132 + select { + case t.output <- time.Now(): + default: + } } // all modifications of the internal state of ThrottleTimer // happen in this method. It is only called from the run goroutine // so we avoid any race conditions func (t *RepeatTimer) processInput(cmd repeatCommand) (shutdown bool) { - fmt.Printf("process: %d\n", cmd) switch cmd { case Reset: - t.timer.Reset(t.dur) + t.ticker.Stop() + t.ticker = time.NewTicker(t.dur) case RQuit: - fmt.Println("got quit") - t.timer.Stop() + t.ticker.Stop() shutdown = true default: panic("unknown command!") diff --git a/common/repeat_timer_test.go b/common/repeat_timer_test.go index 15ca32c3..db53aa61 100644 --- a/common/repeat_timer_test.go +++ b/common/repeat_timer_test.go @@ -39,11 +39,11 @@ func (c *rCounter) Read() { func TestRepeat(test *testing.T) { assert := asrt.New(test) - dur := time.Duration(50) * time.Millisecond + dur := time.Duration(100) * time.Millisecond short := time.Duration(20) * time.Millisecond // delay waits for cnt durations, an a little extra delay := func(cnt int) time.Duration { - return time.Duration(cnt)*dur + time.Duration(5)*time.Millisecond + return time.Duration(cnt)*dur + time.Duration(10)*time.Millisecond } t := NewRepeatTimer("bar", dur) @@ -70,7 +70,7 @@ func TestRepeat(test *testing.T) { // after a stop, nothing more is sent stopped := t.Stop() assert.True(stopped) - time.Sleep(delay(7)) + time.Sleep(delay(2)) assert.Equal(6, c.Count()) // extra calls to stop don't block