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.

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

update mockbeat to reveals a data race when stopping the application #864

Closed

Conversation

cyrilleverrier
Copy link
Contributor

When this patch is applied, running

cd $GOPATH/src/github.com/elastic/beats/libbeat/
RACE_DETECTOR=1 make system-tests

This triggers the following data race detection:

2016/01/27 10:22:46.388070 beat.go:242: DBG  Initializing output plugins
2016/01/27 10:22:46.388145 geolite.go:30: INFO GeoIP disabled: No paths were set under output.geoip.paths
2016/01/27 10:22:46.388186 file.go:48: INFO File output base filename set to: mockbeat
2016/01/27 10:22:46.388219 file.go:60: INFO Rotate every bytes set to: 1024000
2016/01/27 10:22:46.388245 file.go:69: INFO Number of files set to: 7
2016/01/27 10:22:46.388350 outputs.go:129: INFO Activated file as output plugin.
2016/01/27 10:22:46.388379 publish.go:273: DBG  Create output worker
2016/01/27 10:22:46.388552 publish.go:327: DBG  No output is defined to store the topology. The server fields might not be filled.
2016/01/27 10:22:46.388603 publish.go:347: INFO Publisher name: dell
2016/01/27 10:22:46.388925 async.go:91: INFO Flush Interval set to: -1ms
2016/01/27 10:22:46.388954 async.go:99: INFO Max Bulk Size set to: -1
2016/01/27 10:22:46.388993 beat.go:254: INFO Init Beat: mockbeat; Version: 9.9.9
2016/01/27 10:22:46.389394 beat.go:283: INFO mockbeat sucessfully setup. Start running.
2016/01/27 10:22:46.471014 service.go:33: DBG  Received sigterm/sigint, stopping
2016/01/27 10:22:46.471087 beat.go:333: INFO Start exiting beat
2016/01/27 10:22:46.471137 beat.go:301: INFO Stopping Beat
==================
WARNING: DATA RACE
Read by goroutine 8:
  github.com/elastic/beats/libbeat/beat.(*Beat).Stop()
      github.com/elastic/beats/libbeat/beat/_obj/beat.go:303 +0x9a
  github.com/elastic/beats/libbeat/beat.Run()
      github.com/elastic/beats/libbeat/beat/_obj/beat.go:145 +0x144
  github.com/elastic/beats/libbeat.main()
      github.com/elastic/beats/libbeat/_test/_obj_test/libbeat.go:14 +0xd3
  github.com/elastic/beats/libbeat.TestSystem()
      /home/cyrille/code/go/workspace/src/github.com/elastic/beats/libbeat/libbeat_test.go:17 +0x54
  testing.tRunner()
      /usr/lib/go/src/testing/testing.go:456 +0xdc

Previous write by goroutine 9:
  github.com/elastic/beats/libbeat/beat.(*Beat).Run()
      github.com/elastic/beats/libbeat/beat/_obj/beat.go:285 +0x492
  github.com/elastic/beats/libbeat/beat.(*Beat).Start()
      github.com/elastic/beats/libbeat/beat/_obj/beat.go:186 +0x27e
  github.com/elastic/beats/libbeat/beat.Run.func1()
      github.com/elastic/beats/libbeat/beat/_obj/beat.go:127 +0x68

Goroutine 8 (running) created at:
  testing.RunTests()
      /usr/lib/go/src/testing/testing.go:561 +0xaa3
  testing.(*M).Run()
      /usr/lib/go/src/testing/testing.go:494 +0xe4
  main.main()
      github.com/elastic/beats/libbeat/_test/_testmain.go:270 +0x38d

Goroutine 9 (running) created at:
  github.com/elastic/beats/libbeat/beat.Run()
      github.com/elastic/beats/libbeat/beat/_obj/beat.go:139 +0xb6
  github.com/elastic/beats/libbeat.main()
      github.com/elastic/beats/libbeat/_test/_obj_test/libbeat.go:14 +0xd3
  github.com/elastic/beats/libbeat.TestSystem()
      /home/cyrille/code/go/workspace/src/github.com/elastic/beats/libbeat/libbeat_test.go:17 +0x54
  testing.tRunner()
      /usr/lib/go/src/testing/testing.go:456 +0xdc
==================
2016/01/27 10:22:46.471560 beat.go:311: INFO Cleaning up mockbeat before shutting down.
2016/01/27 10:22:46.471679 beat.go:146: INFO Exit beat completed
PASS
coverage: 11.3% of statements in github.com/elastic/beats/libbeat/...
Found 1 data race(s)

The variable b.state in beat.go is read and written by 2 go routines: Run() and the go func started by Run()

@elasticsearch-release
Copy link

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.

@ruflin
Copy link
Member

ruflin commented Jan 27, 2016

To fix this we should probably introduce a function updateState with a mutex.

@cyrilleverrier
Copy link
Contributor Author

Or communicating the state to the main go proc through a dedicated state channel ?

@ruflin
Copy link
Member

ruflin commented Jan 27, 2016

That is probably the more go-like way to do it. It would also allow to change the state for example from inside a beat. Can you add this to your PR?

@andrewkroh
Copy link
Member

jenkins, test it

@ruflin
Copy link
Member

ruflin commented Jan 29, 2016

@cyrilleverrier I took your changes and implemented them in #885 I went with the mutex option as I felt a channel would be overkill. It is easy to change it later to a channel if needed.

ruflin added a commit to ruflin/beats that referenced this pull request Jan 29, 2016
@ruflin
Copy link
Member

ruflin commented Jan 29, 2016

Closing as I think this issue was resolved by #885 Thanks for bringing it up (and provide the solution :-) ).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants