Tags · kidonng/sourcegraph

v3.42.0

sg: fix data race in Runner by wrapping access to progress in mutex (s…

…ourcegraph#38878)

[This build](https://buildkite.com/sourcegraph/sourcegraph/builds/161028#018201db-6317-4457-8226-fbb4628e3381)
failed because of a data race in `TestRunnerInteractive`

    START| RunnerInteractive
    START|   RunnerInteractive/bad_input�[0m
    WARNING: DATA RACE
    Write at 0x00c0001a5d88 by goroutine 93:
      github.com/sourcegraph/sourcegraph/lib/output.(*progressSimple).SetValue()
          /root/buildkite/build/sourcegraph/lib/output/progress_simple.go:32 +0xb3
      github.com/sourcegraph/sourcegraph/lib/output.(*progressWithStatusBarsSimple).SetValue()
          <autogenerated>:1 +0x29
      github.com/sourcegraph/sourcegraph/dev/sg/internal/check.(*Runner[...]).runAllCategoryChecks.func1()
          /root/buildkite/build/sourcegraph/dev/sg/internal/check/runner.go:195 +0x98
          [...]
    Previous write at 0x00c0001a5d88 by goroutine 90:
      github.com/sourcegraph/sourcegraph/lib/output.(*progressSimple).SetValue()
          /root/buildkite/build/sourcegraph/lib/output/progress_simple.go:32 +0xb3
      github.com/sourcegraph/sourcegraph/lib/output.(*progressWithStatusBarsSimple).SetValue()
          <autogenerated>:1 +0x29
      github.com/sourcegraph/sourcegraph/dev/sg/internal/check.(*Runner[...]).runAllCategoryChecks.func1()
          /root/buildkite/build/sourcegraph/dev/sg/internal/check/runner.go:195 +0x98
          [...]
    Goroutine 93 (running) created at:
      github.com/sourcegraph/sourcegraph/lib/group.(*orderedStreamer[...]).initOnce.func1()
          /root/buildkite/build/sourcegraph/lib/group/stream.go:274 +0xc4
      sync.(*Once).doSlow()
          /root/.asdf/installs/golang/1.18.1/go/src/sync/once.go:68 +0x101
          [...]
    Goroutine 90 (running) created at:
      github.com/sourcegraph/sourcegraph/lib/group.(*orderedStreamer[...]).initOnce.func1()
          /root/buildkite/build/sourcegraph/lib/group/stream.go:274 +0xc4
      sync.(*Once).doSlow()
          /root/.asdf/installs/golang/1.18.1/go/src/sync/once.go:68 +0x101
          [...]

Updating the checks counter was safe, since it used `atomic.Float64`,
but multiple writes to `progress` lead to the data race. So this changes
the code to move all the global-state-mutating actions into separate
functions that all use a mutex. That way we don't need `atomic` anymore
and writes to `progress` are protected.