Fix RepeatTimer memory leak (#137)

fix RepeatTimer memory leak (Refs #137)

* test case

* drain channels on reset

Leaking memory:
```
leaktest.go:144: leaktest: leaked goroutine: goroutine 116 [chan send]:
        github.com/tendermint/tmlibs/common.(*RepeatTimer).fireRoutine(0xc42006a410, 0xc4203403c0, 0xc42031b2c0)
                /go/src/github.com/tendermint/tmlibs/common/repeat_timer.go:160 +0x6e
        created by github.com/tendermint/tmlibs/common.(*RepeatTimer).reset
                /go/src/github.com/tendermint/tmlibs/common/repeat_timer.go:196 +0xe9
```

The alternative solution could be draining channels on the client side.

* add one more select instead of draining

thanks to Jae
This commit is contained in:
Anton Kaliaev 2018-02-09 13:31:32 +04:00 committed by GitHub
parent 82ab92da9a
commit 52ce4c20f8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 41 additions and 2 deletions

View File

@ -155,7 +155,11 @@ func (t *RepeatTimer) fireRoutine(ch <-chan time.Time, quit <-chan struct{}) {
for { for {
select { select {
case t_ := <-ch: case t_ := <-ch:
t.ch <- t_ select {
case t.ch <- t_:
case <-quit:
return
}
case <-quit: // NOTE: `t.quit` races. case <-quit: // NOTE: `t.quit` races.
return return
} }
@ -210,7 +214,6 @@ func (t *RepeatTimer) stop() {
t.ticker.Stop() t.ticker.Stop()
t.ticker = nil t.ticker = nil
/* /*
XXX
From https://golang.org/pkg/time/#Ticker: From https://golang.org/pkg/time/#Ticker:
"Stop the ticker to release associated resources" "Stop the ticker to release associated resources"
"After Stop, no more ticks will be sent" "After Stop, no more ticks will be sent"

View File

@ -1,10 +1,12 @@
package common package common
import ( import (
"math/rand"
"sync" "sync"
"testing" "testing"
"time" "time"
"github.com/fortytw2/leaktest"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
) )
@ -102,3 +104,34 @@ func TestRepeatTimer(t *testing.T) {
// Another stop panics. // Another stop panics.
assert.Panics(t, func() { rt.Stop() }) 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()
}
}

2
glide.lock generated
View File

@ -95,6 +95,8 @@ testImports:
version: 346938d642f2ec3594ed81d874461961cd0faa76 version: 346938d642f2ec3594ed81d874461961cd0faa76
subpackages: subpackages:
- spew - spew
- name: github.com/fortytw2/leaktest
version: 3b724c3d7b8729a35bf4e577f71653aec6e53513
- name: github.com/pmezard/go-difflib - name: github.com/pmezard/go-difflib
version: d8ed2627bdf02c080bf22230dbb337003b7aba2d version: d8ed2627bdf02c080bf22230dbb337003b7aba2d
subpackages: subpackages:

View File

@ -35,3 +35,4 @@ testImport:
subpackages: subpackages:
- assert - assert
- require - require
- package: github.com/fortytw2/leaktest