make new() thread safe
okayzed opened this issue · 5 comments
https://github.com/axiomhq/hyperloglog/blob/master/hyperloglog.go#L50 is not threadsafe (compile with -race and run in a multi-threaded program to detect it). this can be worked around by adding a package init function.
func init() {
hash = hashFunc
}
the new() function is not threadsafe because it is modifying a module level variable without locks
I ran into this "race" too. My project runs many daily and pre-checkin tests in race mode and this can lead to a lot of false failures, all that is needed is that two separate goroutines create sketches.
It could be initialized globally once. The tests that screw around with it can just do
hash = nopHash
defer func() { hash = hashFunc }
@RaduBerinde can' t replicate with -race flag sadly. I don't think adding an init is an elegant solution though
if you are using it for testing, you can have a function that lets you inject your dependency (as is the norm). the init() idea was just given as a way of working around the current problem (which is... why did someone decide to set a module variable on every invocation of new())
either way, i've moved to using your loglogbeta repo for now
PS: elegance is over-rated
package main
import hll "github.com/axiomhq/hyperloglog"
func main() {
for i := 0; i < 2; i++ {
go func() {
_ = hll.New()
}()
}
}
compile with: go build -race ./