Merge pull request #116 from ashrayjain/aj/fix-race

Fix race condition in Each()

metrics/registry.go

@@ -214,17 +214,19 @@ func (r *rootRegistry) Subregistry(prefix string, tags ...Tag) Registry {
 func (r *rootRegistry) Each(f MetricVisitor) {
 	// sort names so that iteration order is consistent
 	var sortedNames []string
+	allMetrics := make(map[string]interface{})
 	r.registry.Each(func(name string, metric interface{}) {
 		// filter out the runtime metrics that are defined in the exclude list
 		if _, ok := goRuntimeMetricsToExclude[name]; ok {
 			return
 		}
 		sortedNames = append(sortedNames, name)
+		allMetrics[name] = metric
 	})
 	sort.Strings(sortedNames)
 
 	for _, name := range sortedNames {
-		metric := r.registry.Get(name)
+		metric := allMetrics[name]
 
 		var tags Tags
 		r.idToMetricMutex.RLock()
@@ -238,10 +240,6 @@ func (r *rootRegistry) Each(f MetricVisitor) {
 			sort.Slice(tags, func(i, j int) bool {
 				return tags[i].String() < tags[j].String()
 			})
-		} else {
-			// if metric was not in idToMetricWithTags map, then it is a built-in metric. Trim the prefix since the
-			// registry lookup will automatically prepend the prefix.
-			metric = r.registry.Get(name)
 		}
 		val := ToMetricVal(metric)
 		if val == nil {

metrics/registry_test.go

@@ -243,6 +243,32 @@ func TestRootRegistry_Unregister(t *testing.T) {
 	assert.Equal(t, 0, registrySize(t, registry))
 }
 
+func TestRootRegistry_ConcurrentUnregisterAndEachDoesNotPanic(t *testing.T) {
+	registry := metrics.NewRootMetricsRegistry()
+	registry.Gauge("gauge1").Update(0)
+	registry.Gauge("gauge2").Update(0)
+
+	var firstMetricVisited, metricUnregistered, goRoutineFinished sync.WaitGroup
+	firstMetricVisited.Add(1)
+	metricUnregistered.Add(1)
+	goRoutineFinished.Add(1)
+
+	go func() {
+		registry.Each(metrics.MetricVisitor(func(name string, tags metrics.Tags, metric metrics.MetricVal) {
+			if name == "gauge1" {
+				firstMetricVisited.Done()
+				metricUnregistered.Wait()
+			}
+		}))
+		goRoutineFinished.Done()
+	}()
+
+	firstMetricVisited.Wait()
+	registry.Unregister("gauge2")
+	metricUnregistered.Done()
+	goRoutineFinished.Wait()
+}
+
 func TestRootRegistry_SubregistryWithTags(t *testing.T) {
 	rootRegistry := metrics.NewRootMetricsRegistry()