Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Data race in unit test #2577

Closed
yurishkuro opened this issue Oct 20, 2020 · 0 comments · Fixed by #2583
Closed

Data race in unit test #2577

yurishkuro opened this issue Oct 20, 2020 · 0 comments · Fixed by #2583
Labels
bug good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement

Comments

@yurishkuro
Copy link
Member

This might be flaky behavior as well, I haven't seen this one before. It failed master build.

https://travis-ci.org/github/jaegertracing/jaeger/jobs/735758903

=== RUN   TestTBufferedServer_SendReceive
==================
WARNING: DATA RACE
Write at 0x00c000074850 by goroutine 9:
  runtime.closechan()
      /home/travis/.gimme/versions/go1.15.2.linux.amd64/src/runtime/chan.go:352 +0x0
  github.com/jaegertracing/jaeger/cmd/agent/app/servers.(*TBufferedServer).Stop()
      /home/travis/gopath/src/github.com/jaegertracing/jaeger/cmd/agent/app/servers/tbuffered_server.go:124 +0xb6
  github.com/jaegertracing/jaeger/cmd/agent/app/servers.TestTBufferedServer_SendReceive()
      /home/travis/gopath/src/github.com/jaegertracing/jaeger/cmd/agent/app/servers/tbuffered_server_test.go:83 +0xeaf
  testing.tRunner()
      /home/travis/.gimme/versions/go1.15.2.linux.amd64/src/testing/testing.go:1127 +0x202
Previous read at 0x00c000074850 by goroutine 10:
  runtime.chansend()
      /home/travis/.gimme/versions/go1.15.2.linux.amd64/src/runtime/chan.go:158 +0x0
  github.com/jaegertracing/jaeger/cmd/agent/app/servers.(*TBufferedServer).Serve()
      /home/travis/gopath/src/github.com/jaegertracing/jaeger/cmd/agent/app/servers/tbuffered_server.go:97 +0x264
Goroutine 9 (running) created at:
  testing.(*T).Run()
      /home/travis/.gimme/versions/go1.15.2.linux.amd64/src/testing/testing.go:1178 +0x796
  testing.runTests.func1()
      /home/travis/.gimme/versions/go1.15.2.linux.amd64/src/testing/testing.go:1449 +0xa6
  testing.tRunner()
      /home/travis/.gimme/versions/go1.15.2.linux.amd64/src/testing/testing.go:1127 +0x202
  testing.runTests()
      /home/travis/.gimme/versions/go1.15.2.linux.amd64/src/testing/testing.go:1447 +0x5aa
  testing.(*M).Run()
      /home/travis/.gimme/versions/go1.15.2.linux.amd64/src/testing/testing.go:1357 +0x4eb
  main.main()
      _testmain.go:99 +0x356
Goroutine 10 (finished) created at:
  github.com/jaegertracing/jaeger/cmd/agent/app/servers.TestTBufferedServer_SendReceive()
      /home/travis/gopath/src/github.com/jaegertracing/jaeger/cmd/agent/app/servers/tbuffered_server_test.go:47 +0x209
  testing.tRunner()
      /home/travis/.gimme/versions/go1.15.2.linux.amd64/src/testing/testing.go:1127 +0x202
==================
    testing.go:1042: race detected during execution of test
--- FAIL: TestTBufferedServer_SendReceive (0.01s)

cc @albertteoh
@yurishkuro yurishkuro added bug help wanted Features that maintainers are willing to accept but do not have cycles to implement good first issue Good for beginners labels Oct 20, 2020
chlunde added a commit to chlunde/jaeger that referenced this issue Oct 22, 2020
@chlunde
TBufferedServer: Avoid channel close/send race on Stop
0d43fff 
TBufferedServer.Close() may close the data channel before
Serve detects it. Move chan close() to Serve to prevent
a panic caused by sending on a closed channel.

See also golang/go#27769 (comment)

	=== RUN   TestTBufferedServer_SendReceive
	==================
	WARNING: DATA RACE
	Write at 0x00c000074850 by goroutine 9:
	  runtime.closechan()
	      /home/travis/.gimme/versions/go1.15.2.linux.amd64/src/runtime/chan.go:352 +0x0
	  github.com/jaegertracing/jaeger/cmd/agent/app/servers.(*TBufferedServer).Stop()
	...
	Previous read at 0x00c000074850 by goroutine 10:
	  runtime.chansend()
	      /home/travis/.gimme/versions/go1.15.2.linux.amd64/src/runtime/chan.go:158 +0x0
	  github.com/jaegertracing/jaeger/cmd/agent/app/servers.(*TBufferedServer).Serve()
	      /home/travis/gopath/src/github.com/jaegertracing/jaeger/cmd/agent/app/servers/tbuffered_server.go:97 +0x264
	...
	==================
	    testing.go:1042: race detected during execution of test
	--- FAIL: TestTBufferedServer_SendReceive (0.01s)

Closes jaegertracing#2577

Signed-off-by: Carl Henrik Lunde <[email protected]>
@chlunde chlunde mentioned this issue Oct 22, 2020
Merged
yurishkuro pushed a commit that referenced this issue Oct 25, 2020
@chlunde
TBufferedServer: Avoid channel close/send race on Stop (#2583)
2bce5ad 
TBufferedServer.Close() may close the data channel before
Serve detects it. Move chan close() to Serve to prevent
a panic caused by sending on a closed channel.

See also golang/go#27769 (comment)

	=== RUN   TestTBufferedServer_SendReceive
	==================
	WARNING: DATA RACE
	Write at 0x00c000074850 by goroutine 9:
	  runtime.closechan()
	      /home/travis/.gimme/versions/go1.15.2.linux.amd64/src/runtime/chan.go:352 +0x0
	  github.com/jaegertracing/jaeger/cmd/agent/app/servers.(*TBufferedServer).Stop()
	...
	Previous read at 0x00c000074850 by goroutine 10:
	  runtime.chansend()
	      /home/travis/.gimme/versions/go1.15.2.linux.amd64/src/runtime/chan.go:158 +0x0
	  github.com/jaegertracing/jaeger/cmd/agent/app/servers.(*TBufferedServer).Serve()
	      /home/travis/gopath/src/github.com/jaegertracing/jaeger/cmd/agent/app/servers/tbuffered_server.go:97 +0x264
	...
	==================
	    testing.go:1042: race detected during execution of test
	--- FAIL: TestTBufferedServer_SendReceive (0.01s)

Closes #2577

Signed-off-by: Carl Henrik Lunde <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

TBufferedServer: Avoid channel close/send race on Stop chlunde/jaeger
2 participants
@jpkrohling @yurishkuro