cmd/go/internal/cache: avoid ioutil.WriteFile for writing cache entries
ioutil.WriteFile always truncates the destination file to 0 before
writing, which is inappropriate for unsynchronized, idempotent,
fixed-size files such as the cache entry files here.
Instead, truncate the file only after writing it, so that a second
write will never (even temporarily!) remove the contents of a
preceding write.
Fixes #29667
Change-Id: I16a53ce79d8a23d23580511cb6abd062f54b65ef
Reviewed-on: https://go-review.googlesource.com/c/go/+/188157
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
diff --git a/src/cmd/go/internal/cache/cache.go b/src/cmd/go/internal/cache/cache.go
index 116279c..168ad32 100644
--- a/src/cmd/go/internal/cache/cache.go
+++ b/src/cmd/go/internal/cache/cache.go
@@ -322,7 +322,7 @@
// in verify mode we are double-checking that the cache entries
// are entirely reproducible. As just noted, this may be unrealistic
// in some cases but the check is also useful for shaking out real bugs.
- entry := []byte(fmt.Sprintf("v1 %x %x %20d %20d\n", id, out, size, time.Now().UnixNano()))
+ entry := fmt.Sprintf("v1 %x %x %20d %20d\n", id, out, size, time.Now().UnixNano())
if verify && allowVerify {
old, err := c.get(id)
if err == nil && (old.OutputID != out || old.Size != size) {
@@ -332,7 +332,28 @@
}
}
file := c.fileName(id, "a")
- if err := ioutil.WriteFile(file, entry, 0666); err != nil {
+
+ // Copy file to cache directory.
+ mode := os.O_WRONLY | os.O_CREATE
+ f, err := os.OpenFile(file, mode, 0666)
+ if err != nil {
+ return err
+ }
+ _, err = f.WriteString(entry)
+ if err == nil {
+ // Truncate the file only *after* writing it.
+ // (This should be a no-op, but truncate just in case of previous corruption.)
+ //
+ // This differs from ioutil.WriteFile, which truncates to 0 *before* writing
+ // via os.O_TRUNC. Truncating only after writing ensures that a second write
+ // of the same content to the same file is idempotent, and does not — even
+ // temporarily! — undo the effect of the first write.
+ err = f.Truncate(int64(len(entry)))
+ }
+ if closeErr := f.Close(); err == nil {
+ err = closeErr
+ }
+ if err != nil {
// TODO(bcmills): This Remove potentially races with another go command writing to file.
// Can we eliminate it?
os.Remove(file)
diff --git a/src/cmd/go/testdata/script/mod_list_compiled_concurrent.txt b/src/cmd/go/testdata/script/mod_list_compiled_concurrent.txt
new file mode 100644
index 0000000..b08713d
--- /dev/null
+++ b/src/cmd/go/testdata/script/mod_list_compiled_concurrent.txt
@@ -0,0 +1,41 @@
+env GO111MODULE=on
+
+[short] skip
+
+# Regression test for golang.org/issue/29667:
+# spurious 'failed to cache compiled Go files' errors.
+# This test failed reliably when run with -count=10
+# on a Linux workstation.
+
+env GOCACHE=$WORK/gocache
+mkdir $GOCACHE
+
+go list -json -compiled -test=false -export=false -deps=true -- . &
+go list -json -compiled -test=false -export=false -deps=true -- . &
+go list -json -compiled -test=false -export=false -deps=true -- . &
+go list -json -compiled -test=false -export=false -deps=true -- . &
+go list -json -compiled -test=false -export=false -deps=true -- . &
+go list -json -compiled -test=false -export=false -deps=true -- . &
+go list -json -compiled -test=false -export=false -deps=true -- . &
+go list -json -compiled -test=false -export=false -deps=true -- . &
+go list -json -compiled -test=false -export=false -deps=true -- . &
+go list -json -compiled -test=false -export=false -deps=true -- . &
+go list -json -compiled -test=false -export=false -deps=true -- . &
+go list -json -compiled -test=false -export=false -deps=true -- . &
+go list -json -compiled -test=false -export=false -deps=true -- . &
+go list -json -compiled -test=false -export=false -deps=true -- . &
+go list -json -compiled -test=false -export=false -deps=true -- . &
+go list -json -compiled -test=false -export=false -deps=true -- . &
+go list -json -compiled -test=false -export=false -deps=true -- . &
+go list -json -compiled -test=false -export=false -deps=true -- . &
+go list -json -compiled -test=false -export=false -deps=true -- . &
+go list -json -compiled -test=false -export=false -deps=true -- . &
+
+wait
+
+-- go.mod --
+module sandbox/bar
+-- bar.go --
+package bar
+
+import "C"
