cmd/go/internal/modfetch: push codehost.Repo locking down one level
The modfetch.Repo needs to be safe for use by multiple goroutines,
but the codehost.Repo (a source code repository) was not,
so we added locking around the codehost.Repo methods.
But we might reasonably have multiple modfetch.Repos (modules)
drawing from a single codehost.Repo, especially if we cache
codehost.Repo construction. So push the locking further down.
After this CL, it would still be nice to add locking in package
codehost on a per-directory basis, so that codehost.Run calls
on a given directory are, or at least can be, serialized,
even when they are being invoked from different processes.
To do that we need to work out the right file locking on each
system, which we'll put off for now. (TODO added.)
(Git is already doing file locking to make access from multiple
processes safe at the level of individual commands.)
Change-Id: I9ce4ecf8f8b49cabea50b8663e3cee60ff95e7c9
Reviewed-on: https://go-review.googlesource.com/119775
Reviewed-by: Bryan C. Mills <bcmills@google.com>
diff --git a/vendor/cmd/go/internal/modfetch/codehost/codehost.go b/vendor/cmd/go/internal/modfetch/codehost/codehost.go
index 218bb1a..177be3a 100644
--- a/vendor/cmd/go/internal/modfetch/codehost/codehost.go
+++ b/vendor/cmd/go/internal/modfetch/codehost/codehost.go
@@ -32,6 +32,7 @@
// A Repo represents a code hosting source.
// Typical implementations include local version control repositories,
// remote version control servers, and code hosting sites.
+// A Repo must be safe for simultaneous use by multiple goroutines.
type Repo interface {
// Root returns the import path of the root directory of the repository.
Root() string
diff --git a/vendor/cmd/go/internal/modfetch/coderepo.go b/vendor/cmd/go/internal/modfetch/coderepo.go
index 703b350..ab2a052 100644
--- a/vendor/cmd/go/internal/modfetch/coderepo.go
+++ b/vendor/cmd/go/internal/modfetch/coderepo.go
@@ -15,7 +15,6 @@
"path/filepath"
"regexp"
"strings"
- "sync"
"time"
"cmd/go/internal/modconv"
@@ -32,7 +31,6 @@
code codehost.Repo
codeRoot string
codeDir string
- codeMu sync.Mutex // protects code methods
path string
pathPrefix string
@@ -98,9 +96,7 @@
if r.codeDir != "" {
p = r.codeDir + "/" + p
}
- r.codeMu.Lock()
tags, err := r.code.Tags(p)
- r.codeMu.Unlock()
if err != nil {
return nil, err
}
@@ -130,9 +126,7 @@
if semver.IsValid(codeRev) && r.codeDir != "" {
codeRev = r.codeDir + "/" + codeRev
}
- r.codeMu.Lock()
info, err := r.code.Stat(codeRev)
- r.codeMu.Unlock()
if err != nil {
return nil, err
}
@@ -140,9 +134,7 @@
}
func (r *codeRepo) Latest() (*RevInfo, error) {
- r.codeMu.Lock()
info, err := r.code.Latest()
- r.codeMu.Unlock()
if err != nil {
return nil, err
}
@@ -208,9 +200,7 @@
return rev, "", nil, nil
}
file1 := path.Join(r.codeDir, "go.mod")
- r.codeMu.Lock()
gomod1, err1 := r.code.ReadFile(rev, file1, codehost.MaxGoMod)
- r.codeMu.Unlock()
if err1 != nil {
if os.IsNotExist(err1) {
return "", "", nil, errors.New("missing go.mod")
@@ -229,10 +219,8 @@
// a replace directive.
file1 := path.Join(r.codeDir, "go.mod")
file2 := path.Join(r.codeDir, r.pathMajor[1:], "go.mod")
- r.codeMu.Lock()
gomod1, err1 := r.code.ReadFile(rev, file1, codehost.MaxGoMod)
gomod2, err2 := r.code.ReadFile(rev, file2, codehost.MaxGoMod)
- r.codeMu.Unlock()
if err1 != nil && !os.IsNotExist(err1) {
return "", "", nil, fmt.Errorf("reading %s: %v", file1, err1)
@@ -271,9 +259,7 @@
if gomod != nil {
return gomod, nil
}
- r.codeMu.Lock()
data, err = r.code.ReadFile(rev, path.Join(dir, "go.mod"), codehost.MaxGoMod)
- r.codeMu.Unlock()
if err != nil {
if os.IsNotExist(err) {
return r.legacyGoMod(rev, dir), nil
@@ -300,9 +286,7 @@
mf := new(modfile.File)
mf.AddModuleStmt(r.modPath)
for _, file := range altConfigs {
- r.codeMu.Lock()
data, err := r.code.ReadFile(rev, path.Join(dir, file), codehost.MaxGoMod)
- r.codeMu.Unlock()
if err != nil {
continue
}
@@ -331,9 +315,7 @@
if err != nil {
return "", err
}
- r.codeMu.Lock()
dl, actualDir, err := r.code.ReadZip(rev, dir, codehost.MaxZipFile)
- r.codeMu.Unlock()
if err != nil {
return "", err
}
@@ -471,9 +453,7 @@
}
if !haveLICENSE && subdir != "" {
- r.codeMu.Lock()
data, err := r.code.ReadFile(rev, "LICENSE", codehost.MaxLICENSE)
- r.codeMu.Unlock()
if err == nil {
w, err := zw.Create(r.modPrefix(version) + "/LICENSE")
if err != nil {
diff --git a/vendor/cmd/go/internal/modfetch/gitrepo/fetch.go b/vendor/cmd/go/internal/modfetch/gitrepo/fetch.go
index 8a653a8..e078189 100644
--- a/vendor/cmd/go/internal/modfetch/gitrepo/fetch.go
+++ b/vendor/cmd/go/internal/modfetch/gitrepo/fetch.go
@@ -8,7 +8,6 @@
import (
"archive/zip"
"bytes"
- "cmd/go/internal/modfetch/codehost"
"fmt"
"io"
"io/ioutil"
@@ -19,6 +18,9 @@
"strings"
"sync"
"time"
+
+ "cmd/go/internal/modfetch/codehost"
+ "cmd/go/internal/par"
)
// Repo returns the code repository at the given Git remote reference.
@@ -36,6 +38,27 @@
const workDirType = "git2"
+var repoCache par.Cache
+
+func newRepoCached(remote, root string, localOK bool) (codehost.Repo, error) {
+ type key struct {
+ remote string
+ root string
+ localOK bool
+ }
+ type cached struct {
+ repo codehost.Repo
+ err error
+ }
+
+ c := repoCache.Do(key{remote, root, localOK}, func() interface{} {
+ repo, err := newRepo(remote, root, localOK)
+ return cached{repo, err}
+ }).(cached)
+
+ return c.repo, c.err
+}
+
func newRepo(remote, root string, localOK bool) (codehost.Repo, error) {
r := &repo{remote: remote, root: root, canArchive: true}
if strings.Contains(remote, "://") {
@@ -85,10 +108,12 @@
}
type repo struct {
- remote string
- local bool
- root string
- dir string
+ remote string
+ local bool
+ root string
+ dir string
+
+ mu sync.Mutex // protects canArchive, some git repo state
canArchive bool
refsOnce sync.Once
@@ -270,6 +295,15 @@
return nil, nil, fmt.Errorf("unknown revision %q", rev)
}
+ // Protect r.canArchive.
+ //
+ // Also protects against multiple goroutines running through the
+ // "progressively fetch more and more" sequence at the same time.
+ // TODO(rsc): Add codehost.LockDir and use it for protecting that
+ // sequence, so that multiple processes don't collide.
+ r.mu.Lock()
+ defer r.mu.Unlock()
+
if r.canArchive {
// git archive with --remote requires a ref, not a hash.
// Proceed only if we know a ref for this hash.
