diff --git a/common/repeat_timer.go b/common/repeat_timer.go index cb227199e..dba5fbadd 100644 --- a/common/repeat_timer.go +++ b/common/repeat_timer.go @@ -155,7 +155,11 @@ func (t *RepeatTimer) fireRoutine(ch <-chan time.Time, quit <-chan struct{}) { for { select { case t_ := <-ch: - t.ch <- t_ + select { + case t.ch <- t_: + case <-quit: + return + } case <-quit: // NOTE: `t.quit` races. return } @@ -210,7 +214,6 @@ func (t *RepeatTimer) stop() { t.ticker.Stop() t.ticker = nil /* - XXX From https://golang.org/pkg/time/#Ticker: "Stop the ticker to release associated resources" "After Stop, no more ticks will be sent" diff --git a/common/repeat_timer_test.go b/common/repeat_timer_test.go index 5598922c5..160f4394a 100644 --- a/common/repeat_timer_test.go +++ b/common/repeat_timer_test.go @@ -1,10 +1,12 @@ package common import ( + "math/rand" "sync" "testing" "time" + "github.com/fortytw2/leaktest" "github.com/stretchr/testify/assert" ) @@ -102,3 +104,34 @@ func TestRepeatTimer(t *testing.T) { // Another stop panics. assert.Panics(t, func() { rt.Stop() }) } + +func TestRepeatTimerReset(t *testing.T) { + // check that we are not leaking any go-routines + defer leaktest.Check(t)() + + timer := NewRepeatTimer("test", 20*time.Millisecond) + defer timer.Stop() + + // test we don't receive tick before duration ms. + select { + case <-timer.Chan(): + t.Fatal("did not expect to receive tick") + default: + } + + timer.Reset() + + // test we receive tick after Reset is called + select { + case <-timer.Chan(): + // all good + case <-time.After(40 * time.Millisecond): + t.Fatal("expected to receive tick after reset") + } + + // just random calls + for i := 0; i < 100; i++ { + time.Sleep(time.Duration(rand.Intn(40)) * time.Millisecond) + timer.Reset() + } +} diff --git a/glide.lock b/glide.lock index 10dec980b..a0ada5a4a 100644 --- a/glide.lock +++ b/glide.lock @@ -95,6 +95,8 @@ testImports: version: 346938d642f2ec3594ed81d874461961cd0faa76 subpackages: - spew +- name: github.com/fortytw2/leaktest + version: 3b724c3d7b8729a35bf4e577f71653aec6e53513 - name: github.com/pmezard/go-difflib version: d8ed2627bdf02c080bf22230dbb337003b7aba2d subpackages: diff --git a/glide.yaml b/glide.yaml index b12c72a16..cf3da346b 100644 --- a/glide.yaml +++ b/glide.yaml @@ -35,3 +35,4 @@ testImport: subpackages: - assert - require +- package: github.com/fortytw2/leaktest