fix(tracing): Make Span.Finish a no-op when the span is already finished

tracing.go

@@ -60,6 +60,8 @@ type Span struct { //nolint: maligned // prefer readability over optimal memory
 	contexts map[string]Context
 	// profiler instance if attached, nil otherwise.
 	profiler transactionProfiler
+	// a Once instance to make sure that Finish() is only called once.
+	finishOnce sync.Once
 }
 
 // TraceParentContext describes the context of a (remote) parent span.
@@ -194,12 +196,8 @@ func StartSpan(ctx context.Context, operation string, options ...SpanOption) *Sp
 	return &span
 }
 
-// Finish sets the span's end time, unless already set. If the span is the root
-// of a span tree, Finish sends the span tree to Sentry as a transaction.
-func (s *Span) Finish() {
-	// TODO(tracing): maybe make Finish run at most once, such that
-	// (incorrectly) calling it twice never double sends to Sentry.
-
+// doFinish runs the actual Span.Finish() logic.
+func (s *Span) doFinish() {
 	// For the timing to be correct, the profiler must be stopped before s.EndTime.
 	var profile *profileInfo
 	if s.profiler != nil {
@@ -227,6 +225,15 @@ func (s *Span) Finish() {
 	hub.CaptureEvent(event)
 }
 
+// Finish sets the span's end time, unless already set. If the span is the root
+// of a span tree, Finish sends the span tree to Sentry as a transaction.
+//
+// The logic is executed at most once per span, so that (incorrectly) calling it twice
+// never double sends to Sentry.
+func (s *Span) Finish() {
+	s.finishOnce.Do(s.doFinish)
+}
+
 // Context returns the context containing the span.
 func (s *Span) Context() context.Context { return s.ctx }
 

tracing_test.go

@@ -980,3 +980,28 @@ func TestAdjustingTransactionSourceBeforeSending(t *testing.T) {
 		})
 	}
 }
+
+// This is a regression test for https://github.com/getsentry/sentry-go/issues/587
+// Without the "spans can be finished only once" fix, this test will fail
+// when run with race detection ("-race").
+func TestSpanFinishConcurrentlyWithoutRaces(t *testing.T) {
+	ctx := NewTestContext(ClientOptions{
+		EnableTracing:    true,
+		TracesSampleRate: 1,
+	})
+	transaction := StartTransaction(ctx, "op")
+
+	go func() {
+		for {
+			transaction.Finish()
+		}
+	}()
+
+	go func() {
+		for {
+			transaction.Finish()
+		}
+	}()
+
+	time.Sleep(50 * time.Millisecond)
+}