Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

data race in AddTime #43

Open
arl opened this issue Sep 13, 2018 · 2 comments
Open

data race in AddTime #43

arl opened this issue Sep 13, 2018 · 2 comments

Comments

@arl
Copy link

arl commented Sep 13, 2018

There is a data race in Tachymeter.AddTime

// 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.Count is atomically incremented, but accessing m.Times slice isn't. As such, calling AddTime() from multiple goroutines does triggers 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.

arl added a commit to develersrl/tachymeter that referenced this issue Sep 13, 2018
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
@jamiealquiza
Copy link
Owner

Thanks for opening this (sorry, late - I didn't previously notice this). Will fix this.

@jamiealquiza
Copy link
Owner

Also, did you confirm this in any way that you could share? Calling the AddUint64 from multiple goroutines ensures unique index numbers. You could however write fast enough with a very small m.Times slice that a goroutine that's wrapped around (since this is a sliding window) writes to an index from the previous window. I haven't thought it through yet whether this has undesired consequences.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants