Skip to content

Commit

Permalink
Fix data race in AddTime
Browse files Browse the repository at this point in the history
There was a data race in `Tachymeter.AddTime`.
`m.Count` was atomically incremented, but accessing `m.Times` slice
wasn't. As such, calling `AddTime()` from multiple goroutines triggered
a data race. The higher the number of goroutines and/or the smaller
the value of `m.Size`, the higher the probability of a data race
occurring.

Fixes jamiealquiza#43

Add -race in travis-ci script
  • Loading branch information
arl committed Sep 13, 2018
1 parent 55fe1c4 commit 14a04a0
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 1 deletion.
4 changes: 4 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,7 @@ go:
- "1.10"
- "1.11"
- tip

script:
- go test -v -race -cpu=1,2,4 .

5 changes: 4 additions & 1 deletion tachymeter.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,10 @@ func (m *Tachymeter) Reset() {

// AddTime adds a time.Duration to Tachymeter.
func (m *Tachymeter) AddTime(t time.Duration) {
m.Times[(atomic.AddUint64(&m.Count, 1)-1)%m.Size] = t
m.Lock()
m.Times[m.Count%m.Size] = t
m.Count++
m.Unlock()
}

// SetWallTime optionally sets an elapsed wall time duration.
Expand Down
31 changes: 31 additions & 0 deletions tachymeter_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package tachymeter_test

import (
"sync"
"testing"
"time"

Expand Down Expand Up @@ -54,6 +55,36 @@ func TestAddTime(t *testing.T) {
}
}

func TestAddTimeConcurrent(t *testing.T) {
const (
ngoroutines = 10
ntimes = 100
)

ta := tachymeter.New(&tachymeter.Config{Size: 3})

start := time.Now()

var wg sync.WaitGroup
wg.Add(ngoroutines)

for i := 0; i < ngoroutines; i++ {
go func() {
for j := 0; j < ntimes; j++ {
ta.AddTime(time.Millisecond)
}
wg.Done()
}()
}
wg.Wait()

ta.SetWallTime(time.Since(start))

if ta.Times[0] != time.Millisecond {
t.Fail()
}
}

func TestSetWallTime(t *testing.T) {
ta := tachymeter.New(&tachymeter.Config{Size: 3})

Expand Down

0 comments on commit 14a04a0

Please sign in to comment.