From f1e0ec4dde7e9f16e7f997160736cc4cdfa3a09b Mon Sep 17 00:00:00 2001
From: Kristofer Karlsson <krka@spotify.com>
Date: Thu, 16 Jul 2020 14:33:55 +0200
Subject: [PATCH] Fix race condition with creating lightstep-reporting-thread

Due to lack of synchronization it's possible to spawn multiple
threads and forget the reference to everyone except one.

This means that the forgotten threads will never be interrupted and
keep running forever
---
 .../tracer/shared/AbstractTracer.java         | 44 ++++++++++++-------
 .../lightstep/tracer/shared/ClockState.java   |  4 +-
 2 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/common/src/main/java/com/lightstep/tracer/shared/AbstractTracer.java b/common/src/main/java/com/lightstep/tracer/shared/AbstractTracer.java
index 05cabbcd..787fbbf2 100644
--- a/common/src/main/java/com/lightstep/tracer/shared/AbstractTracer.java
+++ b/common/src/main/java/com/lightstep/tracer/shared/AbstractTracer.java
@@ -243,30 +243,40 @@ private void doStopReporting() {
      */
     private void maybeStartReporting() {
         // We set both reporting/metrics thread in a single step.
-        if (reportingThread != null) {
-            return;
+        boolean buildFirstSpan = false;
+        synchronized (this) {
+            if (reportingThread != null) {
+                return;
+            }
+
+            reportingThread = new Thread(reportingLoop, LightStepConstants.Internal.REPORTING_THREAD_NAME);
+            reportingThread.setDaemon(true);
+            reportingThread.start();
+
+            if (metaEventLoggingEnabled && !firstReportHasRun) {
+                firstReportHasRun = true;
+                buildFirstSpan = true;
+            }
+
+            if (!disableMetricsReporting && safeMetrics != null) {
+                // Can be null, if running on jdk1.7
+                metricsThread = safeMetrics.createMetricsThread(componentName, auth.getAccessToken(),
+                        serviceVersion, metricsUrl, LightStepConstants.Metrics.DEFAULT_INTERVAL_SECS);
+                if (metricsThread != null) {
+                    metricsThread.setDaemon(true);
+                    metricsThread.start();
+                }
+
+            }
         }
-        if (metaEventLoggingEnabled && !firstReportHasRun) {
+
+        if (buildFirstSpan) {
             buildSpan(LightStepConstants.MetaEvents.TracerCreateOperation)
                     .ignoreActiveSpan()
                     .withTag(LightStepConstants.MetaEvents.MetaEventKey, true)
                     .withTag(LightStepConstants.MetaEvents.TracerGuidKey, reporter.getReporterId())
                     .start()
                     .finish();
-            firstReportHasRun = true;
-        }
-        reportingThread = new Thread(reportingLoop, LightStepConstants.Internal.REPORTING_THREAD_NAME);
-        reportingThread.setDaemon(true);
-        reportingThread.start();
-
-        if (!disableMetricsReporting && safeMetrics != null) {
-          // Can be null, if running on jdk1.7
-          metricsThread = safeMetrics.createMetricsThread(componentName, auth.getAccessToken(),
-                    serviceVersion, metricsUrl, LightStepConstants.Metrics.DEFAULT_INTERVAL_SECS);
-          if (metricsThread != null) {
-            metricsThread.setDaemon(true);
-            metricsThread.start();
-          }
         }
     }
 
diff --git a/common/src/main/java/com/lightstep/tracer/shared/ClockState.java b/common/src/main/java/com/lightstep/tracer/shared/ClockState.java
index db4c3e0e..b68036ec 100644
--- a/common/src/main/java/com/lightstep/tracer/shared/ClockState.java
+++ b/common/src/main/java/com/lightstep/tracer/shared/ClockState.java
@@ -154,9 +154,7 @@ long offsetMicros() {
      * in the current offset.
      */
     boolean isReady() {
-        synchronized (mutex) {
-            return samples.size() > 3;
-        }
+        return activeSampleCount() > 3;
     }
 
     int activeSampleCount() {