cmd/go/internal/{modcmd,modload}: lock edits to go.mod

Use an arbitrary lockfile to serialize edits, and use atomic renames
to actually write the go.mod file so that we never drop version
requirements due to a command failing partway through a write.

Multiple invocations of the 'go' command may read the go.mod file
concurrently, and will see some consistent version even if some other
invocation changes it concurrently.

Multiple commands may attempt to write the go.mod file concurrently.
One writer will succeed and write a consistent, complete go.mod file.
The others will detect the changed contents and fail explicitly: it is
not, in general, possible to resolve two conflicting changes to module
requirements, so we surface the problem to the user rather than trying
to solve the problem heuristically.

Updates #26794

Change-Id: Ia1a06a01ef93fa9be664f560eb83bb86b0207443
Reviewed-on: https://go-review.googlesource.com/c/146380
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

src/cmd/go/internal/modcmd/edit.go

@@ -7,6 +7,7 @@
 package modcmd
 
 import (
+	"bytes"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
@@ -15,6 +16,7 @@ import (
 	"strings"
 
 	"cmd/go/internal/base"
+	"cmd/go/internal/modfetch"
 	"cmd/go/internal/modfile"
 	"cmd/go/internal/modload"
 	"cmd/go/internal/module"
@@ -204,17 +206,23 @@ func runEdit(cmd *base.Command, args []string) {
 		return
 	}
 
-	data, err = modFile.Format()
+	out, err := modFile.Format()
 	if err != nil {
 		base.Fatalf("go: %v", err)
 	}
 
 	if *editPrint {
-		os.Stdout.Write(data)
+		os.Stdout.Write(out)
 		return
 	}
 
-	if err := ioutil.WriteFile(gomod, data, 0666); err != nil {
+	unlock := modfetch.SideLock()
+	defer unlock()
+	lockedData, err := ioutil.ReadFile(gomod)
+	if err == nil && !bytes.Equal(lockedData, data) {
+		base.Fatalf("go: go.mod changed during editing; not overwriting")
+	}
+	if err := ioutil.WriteFile(gomod, out, 0666); err != nil {
 		base.Fatalf("go: %v", err)
 	}
 }

src/cmd/go/internal/modload/init.go

@@ -16,6 +16,7 @@ import (
 	"cmd/go/internal/modfile"
 	"cmd/go/internal/module"
 	"cmd/go/internal/mvs"
+	"cmd/go/internal/renameio"
 	"cmd/go/internal/search"
 	"encoding/json"
 	"fmt"
@@ -34,10 +35,11 @@ var (
 	MustUseModules = mustUseModules()
 	initialized    bool
 
-	ModRoot  string
-	modFile  *modfile.File
-	excluded map[module.Version]bool
-	Target   module.Version
+	ModRoot     string
+	modFile     *modfile.File
+	modFileData []byte
+	excluded    map[module.Version]bool
+	Target      module.Version
 
 	gopath string
 
@@ -285,6 +287,7 @@ func InitMod() {
 		base.Fatalf("go: errors parsing go.mod:\n%s\n", err)
 	}
 	modFile = f
+	modFileData = data
 
 	if len(f.Syntax.Stmt) == 0 || f.Module == nil {
 		// Empty mod file. Must add module path.
@@ -579,22 +582,53 @@ func WriteGoMod() {
 		modFile.SetRequire(list)
 	}
 
-	file := filepath.Join(ModRoot, "go.mod")
-	old, _ := ioutil.ReadFile(file)
 	modFile.Cleanup() // clean file after edits
 	new, err := modFile.Format()
 	if err != nil {
 		base.Fatalf("go: %v", err)
 	}
-	if !bytes.Equal(old, new) {
-		if cfg.BuildMod == "readonly" {
-			base.Fatalf("go: updates to go.mod needed, disabled by -mod=readonly")
+
+	// Always update go.sum, even if we didn't change go.mod: we may have
+	// downloaded modules that we didn't have before.
+	modfetch.WriteGoSum()
+
+	if bytes.Equal(new, modFileData) {
+		// We don't need to modify go.mod from what we read previously.
+		// Ignore any intervening edits.
+		return
+	}
+	if cfg.BuildMod == "readonly" {
+		base.Fatalf("go: updates to go.mod needed, disabled by -mod=readonly")
+	}
+
+	unlock := modfetch.SideLock()
+	defer unlock()
+
+	file := filepath.Join(ModRoot, "go.mod")
+	old, err := ioutil.ReadFile(file)
+	if !bytes.Equal(old, modFileData) {
+		if bytes.Equal(old, new) {
+			// Some other process wrote the same go.mod file that we were about to write.
+			modFileData = new
+			return
 		}
-		if err := ioutil.WriteFile(file, new, 0666); err != nil {
-			base.Fatalf("go: %v", err)
+		if err != nil {
+			base.Fatalf("go: can't determine whether go.mod has changed: %v", err)
 		}
+		// The contents of the go.mod file have changed. In theory we could add all
+		// of the new modules to the build list, recompute, and check whether any
+		// module in *our* build list got bumped to a different version, but that's
+		// a lot of work for marginal benefit. Instead, fail the command: if users
+		// want to run concurrent commands, they need to start with a complete,
+		// consistent module definition.
+		base.Fatalf("go: updates to go.mod needed, but contents have changed")
+
 	}
-	modfetch.WriteGoSum()
+
+	if err := renameio.WriteFile(file, new); err != nil {
+		base.Fatalf("error writing go.mod: %v", err)
+	}
+	modFileData = new
 }
 
 func fixVersion(path, vers string) (string, error) {