Skip to content

Fix data races around runner health and build stats

Julius Volz requested to merge juliusv/gitlab-ci-multi-runner:fix/data-races into master

What does this MR do?

It fixes two data races discovered by building with --race enabled and running the runner for a while.

==================
WARNING: DATA RACE
Write at 0x00c4200ba3c0 by goroutine 24:
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands.(*buildsHelper).addBuild()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands/builds_helper.go:79 +0x545
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands.(*RunCommand).processRunner()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands/multi.go:134 +0x70f
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands.(*RunCommand).processRunners()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands/multi.go:156 +0x269

Previous read at 0x00c4200ba3c0 by main goroutine:
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands.(*RunCommand).log()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands/multi.go:59 +0x40
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands.(*RunCommand).runWait()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands/multi.go:299 +0x3f
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands.(*RunCommand).(gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands.runWait)-fm()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands/multi.go:411 +0x41
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/vendor/github.com/ayufan/golang-kardianos-service.(*systemd).Run()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/vendor/github.com/ayufan/golang-kardianos-service/service_systemd_linux.go:138 +0x14a
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands.(*RunCommand).Execute()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands/multi.go:432 +0x412
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/common.(Commander).Execute-fm()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/common/command.go:24 +0x50
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/vendor/github.com/codegangsta/cli.Command.Run()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/vendor/github.com/codegangsta/cli/command.go:127 +0xa8f
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/vendor/github.com/codegangsta/cli.(*App).Run()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/vendor/github.com/codegangsta/cli/app.go:159 +0xb8e
  main.main()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/main.go:58 +0x41b

Goroutine 24 (running) created at:
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands.(*RunCommand).startWorkers()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands/multi.go:172 +0xc1
==================

...and:

==================
WARNING: DATA RACE
Write at 0x00c4200ba440 by main goroutine:
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands.(*RunCommand).runWait()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands/multi.go:302 +0x166
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands.(*RunCommand).(gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands.runWait)-fm()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands/multi.go:411 +0x41
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/vendor/github.com/ayufan/golang-kardianos-service.(*systemd).Run()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/vendor/github.com/ayufan/golang-kardianos-service/service_systemd_linux.go:138 +0x14a
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands.(*RunCommand).Execute()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands/multi.go:432 +0x412
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/common.(Commander).Execute-fm()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/common/command.go:24 +0x50
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/vendor/github.com/codegangsta/cli.Command.Run()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/vendor/github.com/codegangsta/cli/command.go:127 +0xa8f
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/vendor/github.com/codegangsta/cli.(*App).Run()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/vendor/github.com/codegangsta/cli/app.go:159 +0xb8e
  main.main()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/main.go:58 +0x41b

Previous read at 0x00c4200ba440 by goroutine 20:
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands.(*RunCommand).Run()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands/multi.go:319 +0x427

Goroutine 20 (running) created at:
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands.(*RunCommand).Start()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands/multi.go:248 +0x4dd
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/vendor/github.com/ayufan/golang-kardianos-service.(*systemd).Run()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/vendor/github.com/ayufan/golang-kardianos-service/service_systemd_linux.go:129 +0x66
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands.(*RunCommand).Execute()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/commands/multi.go:432 +0x412
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/common.(Commander).Execute-fm()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/common/command.go:24 +0x50
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/vendor/github.com/codegangsta/cli.Command.Run()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/vendor/github.com/codegangsta/cli/command.go:127 +0xa8f
  gitlab.com/gitlab-org/gitlab-ci-multi-runner/vendor/github.com/codegangsta/cli.(*App).Run()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/vendor/github.com/codegangsta/cli/app.go:159 +0xb8e
  main.main()
      /home/julius/gosrc/src/gitlab.com/gitlab-org/gitlab-ci-multi-runner/main.go:58 +0x41b
==================

Why was this MR needed?

These data races may break runner health tracking, as well as the reporting of build queue length.

Are there points in the code the reviewer needs to double check?

None in particular.

Does this MR meet the acceptance criteria?

  • [✘] Documentation created/updated
  • Tests
    • [✘] Added for this feature/bug
    • [✔] All builds are passing
  • [✔] Branch has no merge conflicts with master (if you do - rebase it please)

What are the relevant issue numbers?

None.

Merge request reports