From cedb3873debc80012939e9a999a6fc806729cc22 Mon Sep 17 00:00:00 2001
From: Andrew Wilkins <axw@elastic.co>
Date: Fri, 20 May 2022 08:58:45 +0800
Subject: [PATCH] apm: fix race in DefaultTracer

Fix a race in DefaultTracer which could lead
to two tracers being created, one of which
would be closed. We must hold the write lock
while creating the default tracer, checking
again that it was not set after releasing the
read lock.
---
 tracer.go      | 39 +++++++++++++++++++++++++--------------
 tracer_test.go | 18 ++++++++++++++++++
 2 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/tracer.go b/tracer.go
index 0813f0782..1c45cde6a 100644
--- a/tracer.go
+++ b/tracer.go
@@ -50,30 +50,41 @@ var (
 )
 
 // DefaultTracer returns the default global Tracer, set the first time the
-// function is called. It is configured via environment variables.
+// function is called, or after calling SetDefaultTracer(nil).
 //
-// This will always be initialized to a non-nil value. If any of the
-// environment variables are invalid, the corresponding errors will be logged
-// to stderr and the default values will be used instead.
+// The default tracer is configured via environment variables, and will always
+// be non-nil. If any of the environment variables are invalid, the
+// corresponding errors will be logged to stderr and the default values will be
+// used instead.
 func DefaultTracer() *Tracer {
 	tracerMu.RLock()
-	tracer := defaultTracer
-	tracerMu.RUnlock()
-	if tracer != nil {
+	if defaultTracer != nil {
+		tracer := defaultTracer
+		tracerMu.RUnlock()
 		return tracer
 	}
+	tracerMu.RUnlock()
+
+	tracerMu.Lock()
+	defer tracerMu.Unlock()
+	if defaultTracer != nil {
+		return defaultTracer
+	}
 
 	var opts TracerOptions
 	opts.initDefaults(true)
-	tracer = newTracer(opts)
-	SetDefaultTracer(tracer)
-	return tracer
+	defaultTracer = newTracer(opts)
+	return defaultTracer
 }
 
-// SetDefaultTracer sets the tracer returned by DefaultTracer(). If another
-// tracer has already been initialized, it is closed. Any queued events are not
-// flushed; it is the responsibility of the caller to call
-// DefaultTracer().Flush().
+// SetDefaultTracer sets the tracer returned by DefaultTracer.
+//
+// If a default tracer has already been initialized, it is closed.
+// Any queued events are not flushed; it is the responsibility of the
+// caller to call the default tracer's Flush method first, if needed.
+//
+// Calling SetDefaultTracer(nil) will clear the default tracer,
+// causing DefaultTracer to initialize a new default tracer.
 func SetDefaultTracer(t *Tracer) {
 	tracerMu.Lock()
 	defer tracerMu.Unlock()
diff --git a/tracer_test.go b/tracer_test.go
index 1a0c268cd..c351670ee 100644
--- a/tracer_test.go
+++ b/tracer_test.go
@@ -48,6 +48,24 @@ import (
 	"go.elastic.co/apm/v2/transport/transporttest"
 )
 
+func TestDefaultTracer(t *testing.T) {
+	defer apm.SetDefaultTracer(nil)
+
+	// Call DefaultTracer concurrently to ensure there are
+	// no races in creating the default tracer.
+	tracers := make(chan *apm.Tracer, 1000)
+	for i := 0; i < cap(tracers); i++ {
+		go func() {
+			tracers <- apm.DefaultTracer()
+		}()
+	}
+
+	tracer0 := <-tracers
+	for i := 1; i < cap(tracers); i++ {
+		assert.Same(t, tracer0, <-tracers)
+	}
+}
+
 func TestTracerStats(t *testing.T) {
 	tracer := apmtest.NewDiscardTracer()
 	defer tracer.Close()