Copy Histogram Buckets before sorting (#68)

Summary: go's sort.Sort function will occasionally modify the data
that is passed into it even if that data is already sorted.
(https://play.golang.org/p/QJ8kiJzzVG)

For tally, this means that if a bucket slice is passed into tally, and
tally calls sort.Sort on that slice, the slice will get modified inline
(even if the list is already sorted).

In practice, this means that if a user is using a single slice of buckets
to create multiple tally histograms, there is a race condition where the
shared bucket object could be modified mid-histogram creation.

This PR creates copies of the []float64 and []time.Duration slices before
sorting them, guaranteeing that we won't have a race condition.

histogram.go

@@ -135,12 +135,9 @@ func BucketPairs(buckets Buckets) []BucketPair {
 		}
 	}
 
-	// Sort before iterating to create pairs
-	sort.Sort(buckets)
-
 	var (
-		asValueBuckets    = buckets.AsValues()
-		asDurationBuckets = buckets.AsDurations()
+		asValueBuckets    = copyAndSortValues(buckets.AsValues())
+		asDurationBuckets = copyAndSortDurations(buckets.AsDurations())
 		pairs             = make([]BucketPair, 0, buckets.Len()+2)
 	)
 
@@ -175,6 +172,24 @@ func BucketPairs(buckets Buckets) []BucketPair {
 	return pairs
 }
 
+func copyAndSortValues(values []float64) []float64 {
+	valuesCopy := make([]float64, len(values))
+	for i := range values {
+		valuesCopy[i] = values[i]
+	}
+	sort.Sort(ValueBuckets(valuesCopy))
+	return valuesCopy
+}
+
+func copyAndSortDurations(durations []time.Duration) []time.Duration {
+	durationsCopy := make([]time.Duration, len(durations))
+	for i := range durations {
+		durationsCopy[i] = durations[i]
+	}
+	sort.Sort(DurationBuckets(durationsCopy))
+	return durationsCopy
+}
+
 type bucketPair struct {
 	lowerBoundValue    float64
 	upperBoundValue    float64

histogram_test.go

@@ -165,3 +165,31 @@ func TestMustMakeExponentialDurationBucketsPanicsOnBadFactor(t *testing.T) {
 		MustMakeExponentialDurationBuckets(2*time.Second, 1, 2)
 	})
 }
+
+func TestBucketPairsNoRaceWhenSorted(t *testing.T) {
+	buckets := DurationBuckets{}
+	for i := 0; i < 99; i++ {
+		buckets = append(buckets, time.Duration(i) * time.Second)
+	}
+	newPair := func() {
+		pairs := BucketPairs(buckets)
+		require.Equal(t, 100, len(pairs))
+	}
+	for i := 0; i < 10; i++ {
+		go newPair()
+	}
+}
+
+func TestBucketPairsNoRaceWhenUnsorted(t *testing.T) {
+	buckets := DurationBuckets{}
+	for i := 100; i > 1; i-- {
+		buckets = append(buckets, time.Duration(i) * time.Second)
+	}
+	newPair := func() {
+		pairs := BucketPairs(buckets)
+		require.Equal(t, 100, len(pairs))
+	}
+	for i := 0; i < 10; i++ {
+		go newPair()
+	}
+}