git-codereview: avoid race in loadGerritOrigin

Use a mutex in loadGerritOrigin to avoid race when called in parallel
by "git coderevew pending". Add a new initialized field so that the
code knows when auth has been initialized. Adjust tests accordingly.
The test is simply "go test -race".

Fixes golang/go#43670

Change-Id: Ifb060fca6ed463f1d11a2959d03fca5e14e238c6
Reviewed-on: https://go-review.googlesource.com/c/review/+/287012
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>

1 files changed, 16 insertions, 2 deletions

diff --git a/git-codereview/api.go b/git-codereview/api.go
index c7126f9..afd421f 100644
--- a/git-codereview/api.go
+++ b/git-codereview/api.go
@@ -17,10 +17,13 @@ import (
 	"runtime"
 	"sort"
 	"strings"
+	"sync"
 )
 
 // auth holds cached data about authentication to Gerrit.
 var auth struct {
+	initialized bool
+
 	host    string // "go.googlesource.com"
 	url     string // "https://go-review.googlesource.com"
 	project string // "go", "tools", "crypto", etc
@@ -34,12 +37,21 @@ var auth struct {
 	password    string
 }
 
+// loadGerritOriginMutex is used to control access when initializing auth
+// in loadGerritOrigin, which can be called in parallel by "pending".
+// We use a mutex rather than a sync.Once because the tests clear auth.
+var loadGerritOriginMutex sync.Mutex
+
 // loadGerritOrigin loads the Gerrit host name from the origin remote.
+// This sets auth.{initialized,host,url,project}.
 // If the origin remote does not appear to be a Gerrit server
 // (is missing, is GitHub, is not https, has too many path elements),
 // loadGerritOrigin dies.
 func loadGerritOrigin() {
-	if auth.host != "" {
+	loadGerritOriginMutex.Lock()
+	defer loadGerritOriginMutex.Unlock()
+
+	if auth.initialized {
 		return
 	}
 
@@ -52,6 +64,8 @@ func loadGerritOrigin() {
 	if err != nil {
 		dief("failed to load Gerrit origin: %v", err)
 	}
+
+	auth.initialized = true
 }
 
 // loadGerritOriginInternal does the work of loadGerritOrigin, just extracted out
@@ -279,7 +293,7 @@ func gerritAPI(path string, requestBody []byte, target interface{}) error {
 }
 
 // fullChangeID returns the unambigous Gerrit change ID for the commit c on branch b.
-// The retruned ID has the form project~originbranch~Ihexhexhexhexhex.
+// The returned ID has the form project~originbranch~Ihexhexhexhexhex.
 // See https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#change-id for details.
 func fullChangeID(b *Branch, c *Commit) string {
 	loadGerritOrigin()