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 116279c977d466041b4da1014d23e64e359b7e40..168ad32b0edd17fe4645ff76c35907f82a257564 100644 (file)
--- a/src/cmd/go/internal/cache/cache.go
+++ b/src/cmd/go/internal/cache/cache.go
@@ -322,7 +322,7 @@ func (c *Cache) putIndexEntry(id ActionID, out OutputID, size int64, allowVerify
        // 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 @@ func (c *Cache) putIndexEntry(id ActionID, out OutputID, size int64, allowVerify
                }
        }
        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 (file)
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"