Merge pull request #16 from opentracing/fix_pool

Fix SIGSEGV due to incorrect usage of sync.Pool

debug.go

@@ -16,10 +16,16 @@ type errAssertionFailed struct {
 
 func (s *spanImpl) Lock() {
 	s.Mutex.Lock()
-	s.maybeAssertLocked()
+	s.maybeAssertSanityLocked()
 }
 
-func (s *spanImpl) maybeAssertLocked() {
+func (s *spanImpl) maybeAssertSanityLocked() {
+	if s.tracer == nil {
+		s.Mutex.Unlock()
+		panic(&errAssertionFailed{
+			msg: fmt.Sprintf("span used after Finish()"),
+		})
+	}
 	if s.tracer.Options.DebugAssertSingleGoroutine {
 		startID := curGoroutineID()
 		curID, ok := s.raw.Tags[debugGoroutineIDTag].(uint64)
@@ -29,6 +35,7 @@ func (s *spanImpl) maybeAssertLocked() {
 			return
 		}
 		if startID != curID {
+			s.Mutex.Unlock()
 			panic(&errAssertionFailed{
 				msg: fmt.Sprintf("span started on goroutine %d, but now running on %d", startID, curID),
 			})

span.go

@@ -18,6 +18,10 @@ type spanImpl struct {
 	raw        RawSpan
 }
 
+var spanPool = &sync.Pool{New: func() interface{} {
+	return &spanImpl{}
+}}
+
 func (s *spanImpl) reset() {
 	s.tracer, s.event = nil, nil
 	// Note: Would like to do the following, but then the consumer of RawSpan
@@ -108,24 +112,20 @@ func (s *spanImpl) FinishWithOptions(opts opentracing.FinishOptions) {
 	duration := finishTime.Sub(s.raw.Start)
 
 	s.Lock()
+	defer s.Unlock()
 	if opts.BulkLogData != nil {
 		s.raw.Logs = append(s.raw.Logs, opts.BulkLogData...)
 	}
 	s.raw.Duration = duration
-	s.Unlock()
 
 	s.onFinish(s.raw)
 	s.tracer.Recorder.RecordSpan(s.raw)
 	if s.tracer.Options.DebugAssertUseAfterFinish {
 		// This makes it much more likely to catch a panic on any subsequent
 		// operation since s.tracer is accessed on every call to `Lock`.
-		pool := s.tracer.spanPool
 		s.reset()
-		pool.Put(s)
-	} else {
-		s.tracer.spanPool.Put(s)
 	}
-
+	spanPool.Put(s)
 }
 
 func (s *spanImpl) SetBaggageItem(restrictedKey, val string) opentracing.Span {

tracer.go

@@ -1,7 +1,6 @@
 package basictracer
 
 import (
-	"sync"
 	"time"
 
 	opentracing "github.com/opentracing/opentracing-go"
@@ -79,12 +78,7 @@ func DefaultOptions() Options {
 
 // NewWithOptions creates a customized Tracer.
 func NewWithOptions(opts Options) opentracing.Tracer {
-	rval := &tracerImpl{
-		Options: opts,
-		spanPool: sync.Pool{New: func() interface{} {
-			return &spanImpl{}
-		}},
-	}
+	rval := &tracerImpl{Options: opts}
 	rval.textPropagator = &splitTextPropagator{rval}
 	rval.binaryPropagator = &splitBinaryPropagator{rval}
 	rval.goHTTPPropagator = &goHTTPPropagator{rval.binaryPropagator}
@@ -105,7 +99,6 @@ func New(recorder SpanRecorder) opentracing.Tracer {
 // Implements the `Tracer` interface.
 type tracerImpl struct {
 	Options
-	spanPool           sync.Pool
 	textPropagator     *splitTextPropagator
 	binaryPropagator   *splitBinaryPropagator
 	goHTTPPropagator   *goHTTPPropagator
@@ -122,7 +115,7 @@ func (t *tracerImpl) StartSpan(
 }
 
 func (t *tracerImpl) getSpan() *spanImpl {
-	sp := t.spanPool.Get().(*spanImpl)
+	sp := spanPool.Get().(*spanImpl)
 	sp.reset()
 	return sp
 }