diff options
| author | Bryan C. Mills <bcmills@google.com> | 2022-11-10 17:00:18 -0500 |
|---|---|---|
| committer | Gopher Robot <gobot@golang.org> | 2022-11-11 16:04:21 +0000 |
| commit | fcd14bdcbdfbb5b0c79cfecff95291837836a76d (patch) | |
| tree | c7646aac619f3e9941f04404fe77c34927b0abbb | |
| parent | fffda6b3adfb4d05290c58939a78a99643398ff1 (diff) | |
| download | go-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.go | 19 | ||||
| -rw-r--r-- | src/cmd/go/internal/vcweb/vcweb.go | 3 |
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) } |
