Skip to content

Commit

Permalink
[trace] Fix race between CreateAsyncChild and span.Send
Browse files Browse the repository at this point in the history
`CreateChild` adds a new span to the current span's list of children. `CreateAsyncChild` calls `CreateChild`, then sets the isAsync bool on the span. This can lead to a race caused by simultaneous read of the child span's `isAsync` bool by `Send` as it's being written by `CreateAsyncChild`. To fix this, I refactored  the code a bit to allow us to set `isAsync` before the span is added to the list of child spans.

This doesn't seem to be the same race reported in #38 but it should be fixed either way.
  • Loading branch information
tredman committed Nov 23, 2018
1 parent 87bb646 commit f5f2674
Showing 1 changed file with 16 additions and 13 deletions.
29 changes: 16 additions & 13 deletions trace/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,24 +270,13 @@ func (s *Span) GetParent() *Span {
// outlive the current span (and trace). Async spans are not automatically sent
// when their parent finishes, but are otherwise identical to synchronous spans.
func (s *Span) CreateAsyncChild(ctx context.Context) (context.Context, *Span) {
ctx, newSpan := s.CreateChild(ctx)
newSpan.isAsync = true
return ctx, newSpan
return s.createChildSpan(ctx, true)
}

// Span creates a synchronous child of the current span. Spans must finish
// before their parents.
func (s *Span) CreateChild(ctx context.Context) (context.Context, *Span) {
newSpan := newSpan()
newSpan.parent = s
newSpan.parentID = s.spanID
newSpan.trace = s.trace
newSpan.ev = s.trace.builder.NewEvent()
s.childrenLock.Lock()
s.children = append(s.children, newSpan)
s.childrenLock.Unlock()
ctx = PutSpanInContext(ctx, newSpan)
return ctx, newSpan
return s.createChildSpan(ctx, false)
}

// SerializeHeaders returns the trace ID, current span ID as parent ID, and an
Expand Down Expand Up @@ -370,3 +359,17 @@ func (s *Span) send() {
s.ev.SendPresampled()
}
}

func (s *Span) createChildSpan(ctx context.Context, async bool) (context.Context, *Span) {
newSpan := newSpan()
newSpan.parent = s
newSpan.parentID = s.spanID
newSpan.trace = s.trace
newSpan.ev = s.trace.builder.NewEvent()
newSpan.isAsync = async
s.childrenLock.Lock()
s.children = append(s.children, newSpan)
s.childrenLock.Unlock()
ctx = PutSpanInContext(ctx, newSpan)
return ctx, newSpan
}

0 comments on commit f5f2674

Please sign in to comment.