cmd/go/internal/vcweb: fix a data race in the overview handler

I forgot to lock the scriptResult in the overview handler, and
apparently a cmd/go test is incidentally fetching the overview page at
some point during test execution, triggering the race.

This race was caught almost immediately by the new
linux-amd64-longtest-race builder (see
https://build.golang.org/log/85ab78169a6382a73b1a26c89e64138b387da217).

Updates #27494.

Change-Id: I06ee8d54dba400800284401428ba4a59809983b2
Reviewed-on: https://go-review.googlesource.com/c/go/+/449517
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
This commit is contained in:
Bryan C. Mills 2022-11-10 17:00:18 -05:00 committed by Gopher Robot
parent fffda6b3ad
commit fcd14bdcbd
2 changed files with 22 additions and 0 deletions

View File

@ -20,6 +20,7 @@ import (
"path/filepath"
"strings"
"testing"
"time"
)
var (
@ -87,6 +88,24 @@ func TestScripts(t *testing.T) {
}
srv := httptest.NewServer(s)
// To check for data races in the handler, run the root handler to produce an
// overview of the script status at an arbitrary point during the test.
// (We ignore the output because the expected failure mode is a friendly stack
// dump from the race detector.)
t.Run("overview", func(t *testing.T) {
t.Parallel()
time.Sleep(1 * time.Millisecond) // Give the other handlers time to race.
resp, err := http.Get(srv.URL)
if err == nil {
io.Copy(io.Discard, resp.Body)
resp.Body.Close()
} else {
t.Error(err)
}
})
t.Cleanup(func() {
// The subtests spawned by WalkDir run in parallel. When they complete, this
// Cleanup callback will run. At that point we fetch the root URL (which

View File

@ -383,6 +383,9 @@ func (s *Server) overview(w http.ResponseWriter, r *http.Request) {
status := ""
if ri, ok := s.scriptCache.Load(rel); ok {
r := ri.(*scriptResult)
r.mu.RLock()
defer r.mu.RUnlock()
if !r.hashTime.IsZero() {
hashTime = r.hashTime.Format(time.RFC3339)
}