From 255ce9d1b1a24c02fea15aa94613aa722e644a4b Mon Sep 17 00:00:00 2001
From: Keerthana Selvakumar <keerukeerthana8@gmail.com>
Date: Wed, 26 Aug 2020 00:47:27 +0530
Subject: [PATCH] Adding locks where context is accessed

This commit addresses data race issues where span.context is accessed without locks in methods which can be called concurrently with method setBaggageItem which modifies context

Per issue #526

Signed-off-by: Keerthana Selvakumar <keerukeerthana8@gmail.com>
---
 span.go      | 20 +++++++++++---------
 span_test.go |  1 -
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/span.go b/span.go
index efc37067..8099492a 100644
--- a/span.go
+++ b/span.go
@@ -120,8 +120,8 @@ func (s *Span) setTagInternal(key string, value interface{}, lock bool) opentrac
 
 // SpanContext returns span context
 func (s *Span) SpanContext() SpanContext {
-	s.Lock()
-	defer s.Unlock()
+	s.RLock()
+	defer s.RUnlock()
 	return s.context
 }
 
@@ -345,7 +345,7 @@ func (s *Span) FinishWithOptions(options opentracing.FinishOptions) {
 		decision := s.tracer.sampler.OnFinishSpan(s)
 		s.applySamplingDecision(decision, true)
 	}
-	if s.context.IsSampled() {
+	if s.SpanContext().IsSampled() {
 		s.Lock()
 		s.fixLogsIfDropped()
 		if len(options.LogRecords) > 0 || len(options.BulkLogData) > 0 {
@@ -366,8 +366,8 @@ func (s *Span) FinishWithOptions(options opentracing.FinishOptions) {
 
 // Context implements opentracing.Span API
 func (s *Span) Context() opentracing.SpanContext {
-	s.Lock()
-	defer s.Unlock()
+	s.RLock()
+	defer s.RUnlock()
 	return s.context
 }
 
@@ -427,10 +427,10 @@ func (s *Span) serviceName() string {
 
 func (s *Span) applySamplingDecision(decision SamplingDecision, lock bool) {
 	if !decision.Retryable {
-		s.context.samplingState.setFinal()
+		s.SpanContext().samplingState.setFinal()
 	}
 	if decision.Sample {
-		s.context.samplingState.setSampled()
+		s.SpanContext().samplingState.setSampled()
 		if len(decision.Tags) > 0 {
 			if lock {
 				s.Lock()
@@ -445,12 +445,12 @@ func (s *Span) applySamplingDecision(decision SamplingDecision, lock bool) {
 
 // Span can be written to if it is sampled or the sampling decision has not been finalized.
 func (s *Span) isWriteable() bool {
-	state := s.context.samplingState
+	state := s.SpanContext().samplingState
 	return !state.isFinal() || state.isSampled()
 }
 
 func (s *Span) isSamplingFinalized() bool {
-	return s.context.samplingState.isFinal()
+	return s.SpanContext().samplingState.isFinal()
 }
 
 // setSamplingPriority returns true if the flag was updated successfully, false otherwise.
@@ -469,6 +469,8 @@ func setSamplingPriority(s *Span, value interface{}) bool {
 	if !ok {
 		return false
 	}
+	s.RLock()
+	defer s.RUnlock()
 	if val == 0 {
 		s.context.samplingState.unsetSampled()
 		s.context.samplingState.setFinal()
diff --git a/span_test.go b/span_test.go
index 18083039..cced143d 100644
--- a/span_test.go
+++ b/span_test.go
@@ -363,7 +363,6 @@ func TestSpan_References(t *testing.T) {
 }
 
 func TestSpanContextRaces(t *testing.T) {
-	t.Skip("Skipped: test will panic with -race, see https://github.com/jaegertracing/jaeger-client-go/issues/526")
 	tracer, closer := NewTracer("test", NewConstSampler(true), NewNullReporter())
 	defer closer.Close()