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>

diff --git a/src/cmd/go/internal/vcweb/vcstest/vcstest_test.go b/src/cmd/go/internal/vcweb/vcstest/vcstest_test.go
index d45782d807b62d3ae91424629e804655b960de00..4a6d60039ed0b233b4dbcb995830c78c8dea85ac 100644 (file)
--- 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 5d64b1ee6a835217cddde3584aed2290029d94f5..e1f12827c19edcf5c5bcb24386408719252f9f9d 100644 (file)
--- 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)
                        }