aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBryan C. Mills <bcmills@google.com>2022-11-10 17:00:18 -0500
committerGopher Robot <gobot@golang.org>2022-11-11 16:04:21 +0000
commitfcd14bdcbdfbb5b0c79cfecff95291837836a76d (patch)
treec7646aac619f3e9941f04404fe77c34927b0abbb
parentfffda6b3adfb4d05290c58939a78a99643398ff1 (diff)
downloadgo-fcd14bdcbdfbb5b0c79cfecff95291837836a76d.tar.xz
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>
-rw-r--r--src/cmd/go/internal/vcweb/vcstest/vcstest_test.go19
-rw-r--r--src/cmd/go/internal/vcweb/vcweb.go3
2 files changed, 22 insertions, 0 deletions
diff --git a/src/cmd/go/internal/vcweb/vcstest/vcstest_test.go b/src/cmd/go/internal/vcweb/vcstest/vcstest_test.go
index d45782d807..4a6d60039e 100644
--- a/src/cmd/go/internal/vcweb/vcstest/vcstest_test.go
+++ b/src/cmd/go/internal/vcweb/vcstest/vcstest_test.go
@@ -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
diff --git a/src/cmd/go/internal/vcweb/vcweb.go b/src/cmd/go/internal/vcweb/vcweb.go
index 5d64b1ee6a..e1f12827c1 100644
--- a/src/cmd/go/internal/vcweb/vcweb.go
+++ b/src/cmd/go/internal/vcweb/vcweb.go
@@ -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)
}