fix race condition of the default emitter #173

shogo82148 · 2020-01-14T23:04:33Z

Issue #, if available:

There is a race condition on xray.(*DefaultEmitter).Emit().

Running tool: /usr/local/bin/go test -benchmem -run=^$ github.com/aws/aws-xray-sdk-go/xray -bench ^(BenchmarkDefaultEmitter)$ -v -race

2020-01-15T07:44:55+09:00 [INFO] Emitter using address: 127.0.0.1:2000
goos: darwin
goarch: amd64
pkg: github.com/aws/aws-xray-sdk-go/xray
BenchmarkDefaultEmitter-4   	2020-01-15T07:44:55+09:00 [INFO] Emitter using address: 127.0.0.1:2000
2020-01-15T07:44:55+09:00 [INFO] Emitter using address: 127.0.0.1:2000
==================
2020-01-15T07:44:55+09:00 [INFO] Emitter using address: 127.0.0.1:2000
WARNING: DATA RACE
Write at 0x00c0000cc821 by goroutine 22:
  runtime.slicecopy()
      /usr/local/Cellar/go/1.13.6/libexec/src/runtime/slice.go:197 +0x0
  github.com/aws/aws-xray-sdk-go/xray.(*DefaultEmitter).Emit()
      /Users/shogoichinose/src/github.com/shogo82148/aws-xray-sdk-go/xray/default_emitter.go:81 +0x347
  github.com/aws/aws-xray-sdk-go/xray.BenchmarkDefaultEmitter.func1()
      /Users/shogoichinose/src/github.com/shogo82148/aws-xray-sdk-go/xray/default_emitter_test.go:162 +0x255
  testing.(*B).RunParallel.func1()
      /usr/local/Cellar/go/1.13.6/libexec/src/testing/benchmark.go:742 +0x179

Previous read at 0x00c0000cc820 by goroutine 21:
  syscall.Write()
      /usr/local/Cellar/go/1.13.6/libexec/src/internal/race/race.go:45 +0xab
  internal/poll.(*FD).Write()
      /usr/local/Cellar/go/1.13.6/libexec/src/internal/poll/fd_unix.go:268 +0x1f8
  net.(*netFD).Write()
      /usr/local/Cellar/go/1.13.6/libexec/src/net/fd_unix.go:220 +0x65
  net.(*conn).Write()
      /usr/local/Cellar/go/1.13.6/libexec/src/net/net.go:196 +0xa0
  github.com/aws/aws-xray-sdk-go/xray.(*DefaultEmitter).Emit()
      /Users/shogoichinose/src/github.com/shogo82148/aws-xray-sdk-go/xray/default_emitter.go:81 +0x390
  github.com/aws/aws-xray-sdk-go/xray.BenchmarkDefaultEmitter.func1()
      /Users/shogoichinose/src/github.com/shogo82148/aws-xray-sdk-go/xray/default_emitter_test.go:162 +0x255
  testing.(*B).RunParallel.func1()
      /usr/local/Cellar/go/1.13.6/libexec/src/testing/benchmark.go:742 +0x179

Goroutine 22 (running) created at:
  testing.(*B).RunParallel()
      /usr/local/Cellar/go/1.13.6/libexec/src/testing/benchmark.go:735 +0x256
  github.com/aws/aws-xray-sdk-go/xray.BenchmarkDefaultEmitter()
      /Users/shogoichinose/src/github.com/shogo82148/aws-xray-sdk-go/xray/default_emitter_test.go:153 +0x2df
  testing.(*B).runN()
      /usr/local/Cellar/go/1.13.6/libexec/src/testing/benchmark.go:190 +0x162
  testing.(*B).launch()
      /usr/local/Cellar/go/1.13.6/libexec/src/testing/benchmark.go:320 +0x156

Goroutine 21 (running) created at:
  testing.(*B).RunParallel()
      /usr/local/Cellar/go/1.13.6/libexec/src/testing/benchmark.go:735 +0x256
  github.com/aws/aws-xray-sdk-go/xray.BenchmarkDefaultEmitter()
      /Users/shogoichinose/src/github.com/shogo82148/aws-xray-sdk-go/xray/default_emitter_test.go:153 +0x2df
  testing.(*B).runN()
      /usr/local/Cellar/go/1.13.6/libexec/src/testing/benchmark.go:190 +0x162
  testing.(*B).launch()
      /usr/local/Cellar/go/1.13.6/libexec/src/testing/benchmark.go:320 +0x156
==================
==================
WARNING: DATA RACE
Write at 0x00c0000cc821 by goroutine 23:
  runtime.slicecopy()
      /usr/local/Cellar/go/1.13.6/libexec/src/runtime/slice.go:197 +0x0
  github.com/aws/aws-xray-sdk-go/xray.(*DefaultEmitter).Emit()
      /Users/shogoichinose/src/github.com/shogo82148/aws-xray-sdk-go/xray/default_emitter.go:81 +0x347
  github.com/aws/aws-xray-sdk-go/xray.BenchmarkDefaultEmitter.func1()
      /Users/shogoichinose/src/github.com/shogo82148/aws-xray-sdk-go/xray/default_emitter_test.go:162 +0x255
  testing.(*B).RunParallel.func1()
      /usr/local/Cellar/go/1.13.6/libexec/src/testing/benchmark.go:742 +0x179

Previous read at 0x00c0000cc820 by goroutine 21:
  syscall.Write()
      /usr/local/Cellar/go/1.13.6/libexec/src/internal/race/race.go:45 +0xab
  internal/poll.(*FD).Write()
      /usr/local/Cellar/go/1.13.6/libexec/src/internal/poll/fd_unix.go:268 +0x1f8
  net.(*netFD).Write()
      /usr/local/Cellar/go/1.13.6/libexec/src/net/fd_unix.go:220 +0x65
  net.(*conn).Write()
      /usr/local/Cellar/go/1.13.6/libexec/src/net/net.go:196 +0xa0
  github.com/aws/aws-xray-sdk-go/xray.(*DefaultEmitter).Emit()
      /Users/shogoichinose/src/github.com/shogo82148/aws-xray-sdk-go/xray/default_emitter.go:81 +0x390
  github.com/aws/aws-xray-sdk-go/xray.BenchmarkDefaultEmitter.func1()
      /Users/shogoichinose/src/github.com/shogo82148/aws-xray-sdk-go/xray/default_emitter_test.go:162 +0x255
  testing.(*B).RunParallel.func1()
      /usr/local/Cellar/go/1.13.6/libexec/src/testing/benchmark.go:742 +0x179

Goroutine 23 (running) created at:
  testing.(*B).RunParallel()
      /usr/local/Cellar/go/1.13.6/libexec/src/testing/benchmark.go:735 +0x256
  github.com/aws/aws-xray-sdk-go/xray.BenchmarkDefaultEmitter()
      /Users/shogoichinose/src/github.com/shogo82148/aws-xray-sdk-go/xray/default_emitter_test.go:153 +0x2df
  testing.(*B).runN()
      /usr/local/Cellar/go/1.13.6/libexec/src/testing/benchmark.go:190 +0x162
  testing.(*B).launch()
      /usr/local/Cellar/go/1.13.6/libexec/src/testing/benchmark.go:320 +0x156

Goroutine 21 (running) created at:
  testing.(*B).RunParallel()
      /usr/local/Cellar/go/1.13.6/libexec/src/testing/benchmark.go:735 +0x256
  github.com/aws/aws-xray-sdk-go/xray.BenchmarkDefaultEmitter()
      /Users/shogoichinose/src/github.com/shogo82148/aws-xray-sdk-go/xray/default_emitter_test.go:153 +0x2df
  testing.(*B).runN()
      /usr/local/Cellar/go/1.13.6/libexec/src/testing/benchmark.go:190 +0x162
  testing.(*B).launch()
      /usr/local/Cellar/go/1.13.6/libexec/src/testing/benchmark.go:320 +0x156
==================
2020-01-15T07:44:55+09:00 [INFO] Emitter using address: 127.0.0.1:2000
--- FAIL: BenchmarkDefaultEmitter-4
    /Users/shogoichinose/src/github.com/shogo82148/aws-xray-sdk-go/xray/benchmark.go:196: race detected during execution of benchmark
FAIL
exit status 1
FAIL	github.com/aws/aws-xray-sdk-go/xray	0.420s
FAIL
Error: Benchmarks failed.

Description of changes:

I removed append(Header, p...).
p is written to Header[len(Header):cap(Header)] if there is enough capacity.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

bhautikpip · 2020-01-14T23:52:28Z

Hi @shogo82148,

Looks like you have increased the size of buffer in default_emitter.go. Any ideas on why there is a data race and how can we remove it ?

shogo82148 · 2020-01-16T04:21:01Z

This pull request removes the race condition.

bhautikpip · 2020-02-25T23:03:32Z

xray/default_emitter.go

@@ -77,11 +81,15 @@ func (de *DefaultEmitter) Emit(seg *Segment) {
 				return
 			}
 		}
+		buf := de.buf[:0]
+		buf = append(buf, Header...)


We can also remove this data race by declaring Header inside this function instead of declaring Header as global variable or make it a const.

bhautikpip · 2020-03-06T22:01:51Z

Hi @shogo82148,

Merged a fix for this issue. As per my understanding the issue was we have declared Header as a global variable and that was causing data race when there are multiple goroutines. Here's a PR that address this issue (#200). I am going to close this PR if that's fine.

shogo82148 · 2020-03-07T08:29:33Z

#200 looks good to me. thanks.

shogo82148 added 2 commits January 15, 2020 08:02

fix race condition of the default emitter

b9bfc56

add a comment that points the issue

3f291ce

fix typo of \n

430586c

bhautikpip self-requested a review February 25, 2020 23:02

bhautikpip reviewed Mar 2, 2020

View reviewed changes

shogo82148 closed this Mar 7, 2020

shogo82148 deleted the fix-race-condition-of-emmitter branch March 7, 2020 08:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix race condition of the default emitter #173

fix race condition of the default emitter #173

shogo82148 commented Jan 14, 2020

bhautikpip commented Jan 14, 2020 •

edited

Loading

shogo82148 commented Jan 16, 2020

bhautikpip Feb 25, 2020

bhautikpip commented Mar 6, 2020

shogo82148 commented Mar 7, 2020

fix race condition of the default emitter #173

fix race condition of the default emitter #173

Conversation

shogo82148 commented Jan 14, 2020

bhautikpip commented Jan 14, 2020 • edited Loading

shogo82148 commented Jan 16, 2020

bhautikpip Feb 25, 2020

Choose a reason for hiding this comment

bhautikpip commented Mar 6, 2020

shogo82148 commented Mar 7, 2020

bhautikpip commented Jan 14, 2020 •

edited

Loading