diff --git a/_examples/basic/main.go b/_examples/basic/main.go index 9833cf456..bb1c51da6 100644 --- a/_examples/basic/main.go +++ b/_examples/basic/main.go @@ -3,9 +3,9 @@ // // Try it by running: // -// go run main.go -// go run main.go https://sentry.io -// go run main.go bad-url +// go run main.go +// go run main.go https://sentry.io +// go run main.go bad-url // // To actually report events to Sentry, set the DSN either by editing the // appropriate line below or setting the environment variable SENTRY_DSN to diff --git a/_examples/recover-repanic/main.go b/_examples/recover-repanic/main.go index 5344f0933..a35bc7482 100644 --- a/_examples/recover-repanic/main.go +++ b/_examples/recover-repanic/main.go @@ -5,7 +5,7 @@ // // Try it by running: // -// go run main.go +// go run main.go // // To actually report events to Sentry, set the DSN either by editing the // appropriate line below or setting the environment variable SENTRY_DSN to diff --git a/scope.go b/scope.go index 03602bd65..52cb76192 100644 --- a/scope.go +++ b/scope.go @@ -287,7 +287,7 @@ func (scope *Scope) Clone() *Scope { clone.tags[key] = value } for key, value := range scope.contexts { - clone.contexts[key] = value + clone.contexts[key] = cloneContext(value) } for key, value := range scope.extra { clone.extra[key] = value @@ -350,7 +350,7 @@ func (scope *Scope) ApplyToEvent(event *Event, hint *EventHint) *Event { // Ensure we are not overwriting event fields if _, ok := event.Contexts[key]; !ok { - event.Contexts[key] = value + event.Contexts[key] = cloneContext(value) } } } @@ -404,3 +404,16 @@ func (scope *Scope) ApplyToEvent(event *Event, hint *EventHint) *Event { return event } + +// cloneContext returns a new context with keys and values copied from the passed one. +// +// Note: a new Context (map) is returned, but the function does NOT do +// a proper deep copy: if some context values are pointer types (e.g. maps), +// they won't be properly copied. +func cloneContext(c Context) Context { + res := Context{} + for k, v := range c { + res[k] = v + } + return res +} diff --git a/scope_test.go b/scope_test.go index eb295598b..b2ca37465 100644 --- a/scope_test.go +++ b/scope_test.go @@ -618,3 +618,25 @@ func TestEventProcessorsAddEventProcessor(t *testing.T) { t.Error("event should be dropped") } } + +func TestCloneContext(t *testing.T) { + context := Context{ + "key1": "value1", + "key2": []string{"s1", "s2"}, + } + + clone := cloneContext(context) + + // Value-wise they should be identical + assertEqual(t, context, clone) + // ..but it shouldn't be the same map + if &context == &clone { + t.Error("original and cloned context should be different objects") + } + + sliceOriginal := context["key2"].([]string) + sliceClone := clone["key2"].([]string) + if &sliceOriginal[0] != &sliceClone[0] { + t.Error("complex values are not supposed to be copied") + } +} diff --git a/tracing.go b/tracing.go index 4c6747353..10fc88eae 100644 --- a/tracing.go +++ b/tracing.go @@ -515,7 +515,7 @@ func (s *Span) toEvent() *Event { contexts := map[string]Context{} for k, v := range s.contexts { - contexts[k] = v + contexts[k] = cloneContext(v) } contexts["trace"] = s.traceContext().Map() diff --git a/tracing_test.go b/tracing_test.go index ca3762a1d..dc7fd9d75 100644 --- a/tracing_test.go +++ b/tracing_test.go @@ -10,6 +10,7 @@ import ( "math" "net/http" "reflect" + "sync" "testing" "time" @@ -891,6 +892,49 @@ func TestDeprecatedSpanOptionTransctionSource(t *testing.T) { StartSpan(context.Background(), "op", TransctionSource("src")) } +// This test checks that there are no concurrent reads/writes to +// substructures in scope.contexts. +// See https://github.com/getsentry/sentry-go/issues/570 for more details. +func TestConcurrentContextAccess(t *testing.T) { + ctx := NewTestContext(ClientOptions{ + EnableTracing: true, + TracesSampleRate: 1, + }) + hub := GetHubFromContext(ctx) + + const writersNum = 200 + + // Unbuffered channel, writing to it will be block if nobody reads + c := make(chan *Span) + + // Start writers + for i := 0; i < writersNum; i++ { + go func() { + transaction := StartTransaction(ctx, "test") + c <- transaction + hub.Scope().SetContext("device", Context{"test": "bla"}) + }() + } + + var wg sync.WaitGroup + wg.Add(writersNum) + + // Start readers + go func() { + for transaction := range c { + transaction := transaction + go func() { + defer wg.Done() + // While finalizing every transaction, scope.Contexts and Event.Contexts fields + // will be accessed, e.g. in environmentIntegration.processor() + transaction.Finish() + }() + } + }() + + wg.Wait() +} + func TestAdjustingTransactionSourceBeforeSending(t *testing.T) { tests := []struct { name string