From 003be44413f74c6f7f4e2c01093ce61159e0f71b Mon Sep 17 00:00:00 2001 From: shendley Date: Thu, 4 May 2017 08:05:08 -0400 Subject: [PATCH] add synchronization around removeGauge/addGauge block to prevent exceptions during startup Also done on another fork of project but not PRed https://github.com/spredfast/finagle-metrics/commit/e03d5aeb2c6e751c5fcee6addef693ef6c6df901 --- .../metrics/MetricsStatsReceiver.scala | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/main/scala/com/twitter/finagle/metrics/MetricsStatsReceiver.scala b/src/main/scala/com/twitter/finagle/metrics/MetricsStatsReceiver.scala index 07eb686..1298a3d 100644 --- a/src/main/scala/com/twitter/finagle/metrics/MetricsStatsReceiver.scala +++ b/src/main/scala/com/twitter/finagle/metrics/MetricsStatsReceiver.scala @@ -13,16 +13,21 @@ object MetricsStatsReceiver { } private[metrics] case class MetricGauge(name: String)(f: => Float) extends Gauge { - // remove old gauge's value before adding a new one - metrics.getGauges(new MetricFilter() { - override def matches(metricName: String, metric: Metric): Boolean = - metricName == name - }).asScala - .foreach { case (gaugeName, _) => metrics.remove(gaugeName) } - - metrics.register(name, new MGauge[Float]() { - override def getValue(): Float = f - }) + // we need to synchronize so this is safe with multiple threads. Ideally the callers are themselves + // synchronized so they don't overwrite each others gauges but until they are we should protect + // ourselves from that race condition. + synchronized { + // remove old gauge's value before adding a new one + metrics.getGauges(new MetricFilter() { + override def matches(metricName: String, metric: Metric): Boolean = + metricName == name + }).asScala + .foreach { case (gaugeName, _) => metrics.remove(gaugeName) } + + metrics.register(name, new MGauge[Float]() { + override def getValue(): Float = f + }) + } override def remove(): Unit = metrics.remove(name) }