diff --git a/src/cmd/go/internal/modfetch/cache.go b/src/cmd/go/internal/modfetch/cache.go index d6774e1ce1..776def7cbc 100644 --- a/src/cmd/go/internal/modfetch/cache.go +++ b/src/cmd/go/internal/modfetch/cache.go @@ -11,8 +11,10 @@ import ( "fmt" "io" "io/fs" + "math/rand" "os" "path/filepath" + "strconv" "strings" "sync" @@ -21,7 +23,7 @@ import ( "cmd/go/internal/lockedfile" "cmd/go/internal/modfetch/codehost" "cmd/go/internal/par" - "cmd/go/internal/renameio" + "cmd/go/internal/robustio" "golang.org/x/mod/module" "golang.org/x/mod/semver" @@ -543,7 +545,7 @@ func readDiskCache(path, rev, suffix string) (file string, data []byte, err erro if err != nil { return "", nil, errNotCached } - data, err = renameio.ReadFile(file) + data, err = robustio.ReadFile(file) if err != nil { return file, nil, errNotCached } @@ -580,7 +582,29 @@ func writeDiskCache(file string, data []byte) error { return err } - if err := renameio.WriteFile(file, data, 0666); err != nil { + // Write the file to a temporary location, and then rename it to its final + // path to reduce the likelihood of a corrupt file existing at that final path. + f, err := tempFile(filepath.Dir(file), filepath.Base(file), 0666) + if err != nil { + return err + } + defer func() { + // Only call os.Remove on f.Name() if we failed to rename it: otherwise, + // some other process may have created a new file with the same name after + // the rename completed. + if err != nil { + f.Close() + os.Remove(f.Name()) + } + }() + + if _, err := f.Write(data); err != nil { + return err + } + if err := f.Close(); err != nil { + return err + } + if err := robustio.Rename(f.Name(), file); err != nil { return err } @@ -590,6 +614,19 @@ func writeDiskCache(file string, data []byte) error { return nil } +// tempFile creates a new temporary file with given permission bits. +func tempFile(dir, prefix string, perm fs.FileMode) (f *os.File, err error) { + for i := 0; i < 10000; i++ { + name := filepath.Join(dir, prefix+strconv.Itoa(rand.Intn(1000000000))+".tmp") + f, err = os.OpenFile(name, os.O_RDWR|os.O_CREATE|os.O_EXCL, perm) + if os.IsExist(err) { + continue + } + break + } + return +} + // rewriteVersionList rewrites the version list in dir // after a new *.mod file has been written. func rewriteVersionList(dir string) (err error) { diff --git a/src/cmd/go/internal/modfetch/fetch.go b/src/cmd/go/internal/modfetch/fetch.go index 4ee490c5ea..e40593abae 100644 --- a/src/cmd/go/internal/modfetch/fetch.go +++ b/src/cmd/go/internal/modfetch/fetch.go @@ -24,7 +24,6 @@ import ( "cmd/go/internal/cfg" "cmd/go/internal/lockedfile" "cmd/go/internal/par" - "cmd/go/internal/renameio" "cmd/go/internal/robustio" "cmd/go/internal/trace" @@ -223,11 +222,10 @@ func downloadZip(ctx context.Context, mod module.Version, zipfile string) (err e // Clean up any remaining tempfiles from previous runs. // This is only safe to do because the lock file ensures that their // writers are no longer active. - for _, base := range []string{zipfile, zipfile + "hash"} { - if old, err := filepath.Glob(renameio.Pattern(base)); err == nil { - for _, path := range old { - os.Remove(path) // best effort - } + tmpPattern := filepath.Base(zipfile) + "*.tmp" + if old, err := filepath.Glob(filepath.Join(filepath.Dir(zipfile), tmpPattern)); err == nil { + for _, path := range old { + os.Remove(path) // best effort } } @@ -242,7 +240,7 @@ func downloadZip(ctx context.Context, mod module.Version, zipfile string) (err e // contents of the file (by hashing it) before we commit it. Because the file // is zip-compressed, we need an actual file — or at least an io.ReaderAt — to // validate it: we can't just tee the stream as we write it. - f, err := os.CreateTemp(filepath.Dir(zipfile), filepath.Base(renameio.Pattern(zipfile))) + f, err := os.CreateTemp(filepath.Dir(zipfile), tmpPattern) if err != nil { return err } diff --git a/src/cmd/go/internal/renameio/renameio.go b/src/cmd/go/internal/renameio/renameio.go deleted file mode 100644 index 811f4573a0..0000000000 --- a/src/cmd/go/internal/renameio/renameio.go +++ /dev/null @@ -1,88 +0,0 @@ -// Copyright 2018 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// Package renameio writes files atomically by renaming temporary files. -package renameio - -import ( - "bytes" - "io" - "io/fs" - "math/rand" - "os" - "path/filepath" - "strconv" - - "cmd/go/internal/robustio" -) - -const patternSuffix = ".tmp" - -// Pattern returns a glob pattern that matches the unrenamed temporary files -// created when writing to filename. -func Pattern(filename string) string { - return filepath.Join(filepath.Dir(filename), filepath.Base(filename)+patternSuffix) -} - -// WriteFile is like os.WriteFile, but first writes data to an arbitrary -// file in the same directory as filename, then renames it atomically to the -// final name. -// -// That ensures that the final location, if it exists, is always a complete file. -func WriteFile(filename string, data []byte, perm fs.FileMode) (err error) { - f, err := tempFile(filepath.Dir(filename), filepath.Base(filename), perm) - if err != nil { - return err - } - defer func() { - // Only call os.Remove on f.Name() if we failed to rename it: otherwise, - // some other process may have created a new file with the same name after - // that. - if err != nil { - f.Close() - os.Remove(f.Name()) - } - }() - - if _, err := io.Copy(f, bytes.NewReader(data)); err != nil { - return err - } - // Sync the file before renaming it: otherwise, after a crash the reader may - // observe a 0-length file instead of the actual contents. - // See https://golang.org/issue/22397#issuecomment-380831736. - if err := f.Sync(); err != nil { - return err - } - if err := f.Close(); err != nil { - return err - } - - return robustio.Rename(f.Name(), filename) -} - -// ReadFile is like os.ReadFile, but on Windows retries spurious errors that -// may occur if the file is concurrently replaced. -// -// Errors are classified heuristically and retries are bounded, so even this -// function may occasionally return a spurious error on Windows. -// If so, the error will likely wrap one of: -// - syscall.ERROR_ACCESS_DENIED -// - syscall.ERROR_FILE_NOT_FOUND -// - internal/syscall/windows.ERROR_SHARING_VIOLATION -func ReadFile(filename string) ([]byte, error) { - return robustio.ReadFile(filename) -} - -// tempFile creates a new temporary file with given permission bits. -func tempFile(dir, prefix string, perm fs.FileMode) (f *os.File, err error) { - for i := 0; i < 10000; i++ { - name := filepath.Join(dir, prefix+strconv.Itoa(rand.Intn(1000000000))+patternSuffix) - f, err = os.OpenFile(name, os.O_RDWR|os.O_CREATE|os.O_EXCL, perm) - if os.IsExist(err) { - continue - } - break - } - return -} diff --git a/src/cmd/go/internal/renameio/renameio_test.go b/src/cmd/go/internal/renameio/renameio_test.go deleted file mode 100644 index 1c8d7e311d..0000000000 --- a/src/cmd/go/internal/renameio/renameio_test.go +++ /dev/null @@ -1,161 +0,0 @@ -// Copyright 2019 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -//go:build !plan9 -// +build !plan9 - -package renameio - -import ( - "encoding/binary" - "errors" - "internal/testenv" - "math/rand" - "os" - "path/filepath" - "runtime" - "strings" - "sync" - "sync/atomic" - "syscall" - "testing" - "time" - - "cmd/go/internal/robustio" -) - -func TestConcurrentReadsAndWrites(t *testing.T) { - if runtime.GOOS == "darwin" && strings.HasSuffix(testenv.Builder(), "-10_14") { - testenv.SkipFlaky(t, 33041) - } - - dir, err := os.MkdirTemp("", "renameio") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(dir) - path := filepath.Join(dir, "blob.bin") - - const chunkWords = 8 << 10 - buf := make([]byte, 2*chunkWords*8) - for i := uint64(0); i < 2*chunkWords; i++ { - binary.LittleEndian.PutUint64(buf[i*8:], i) - } - - var attempts int64 = 128 - if !testing.Short() { - attempts *= 16 - } - const parallel = 32 - - var sem = make(chan bool, parallel) - - var ( - writeSuccesses, readSuccesses int64 // atomic - writeErrnoSeen, readErrnoSeen sync.Map - ) - - for n := attempts; n > 0; n-- { - sem <- true - go func() { - defer func() { <-sem }() - - time.Sleep(time.Duration(rand.Intn(100)) * time.Microsecond) - offset := rand.Intn(chunkWords) - chunk := buf[offset*8 : (offset+chunkWords)*8] - if err := WriteFile(path, chunk, 0666); err == nil { - atomic.AddInt64(&writeSuccesses, 1) - } else if robustio.IsEphemeralError(err) { - var ( - errno syscall.Errno - dup bool - ) - if errors.As(err, &errno) { - _, dup = writeErrnoSeen.LoadOrStore(errno, true) - } - if !dup { - t.Logf("ephemeral error: %v", err) - } - } else { - t.Errorf("unexpected error: %v", err) - } - - time.Sleep(time.Duration(rand.Intn(100)) * time.Microsecond) - data, err := robustio.ReadFile(path) - if err == nil { - atomic.AddInt64(&readSuccesses, 1) - } else if robustio.IsEphemeralError(err) { - var ( - errno syscall.Errno - dup bool - ) - if errors.As(err, &errno) { - _, dup = readErrnoSeen.LoadOrStore(errno, true) - } - if !dup { - t.Logf("ephemeral error: %v", err) - } - return - } else { - t.Errorf("unexpected error: %v", err) - return - } - - if len(data) != 8*chunkWords { - t.Errorf("read %d bytes, but each write is a %d-byte file", len(data), 8*chunkWords) - return - } - - u := binary.LittleEndian.Uint64(data) - for i := 1; i < chunkWords; i++ { - next := binary.LittleEndian.Uint64(data[i*8:]) - if next != u+1 { - t.Errorf("wrote sequential integers, but read integer out of sequence at offset %d", i) - return - } - u = next - } - }() - } - - for n := parallel; n > 0; n-- { - sem <- true - } - - var minWriteSuccesses int64 = attempts - if runtime.GOOS == "windows" { - // Windows produces frequent "Access is denied" errors under heavy rename load. - // As long as those are the only errors and *some* of the writes succeed, we're happy. - minWriteSuccesses = attempts / 4 - } - - if writeSuccesses < minWriteSuccesses { - t.Errorf("%d (of %d) writes succeeded; want ≥ %d", writeSuccesses, attempts, minWriteSuccesses) - } else { - t.Logf("%d (of %d) writes succeeded (ok: ≥ %d)", writeSuccesses, attempts, minWriteSuccesses) - } - - var minReadSuccesses int64 = attempts - - switch runtime.GOOS { - case "windows": - // Windows produces frequent "Access is denied" errors under heavy rename load. - // As long as those are the only errors and *some* of the reads succeed, we're happy. - minReadSuccesses = attempts / 4 - - case "darwin", "ios": - // The filesystem on certain versions of macOS (10.14) and iOS (affected - // versions TBD) occasionally fail with "no such file or directory" errors. - // See https://golang.org/issue/33041 and https://golang.org/issue/42066. - // The flake rate is fairly low, so ensure that at least 75% of attempts - // succeed. - minReadSuccesses = attempts - (attempts / 4) - } - - if readSuccesses < minReadSuccesses { - t.Errorf("%d (of %d) reads succeeded; want ≥ %d", readSuccesses, attempts, minReadSuccesses) - } else { - t.Logf("%d (of %d) reads succeeded (ok: ≥ %d)", readSuccesses, attempts, minReadSuccesses) - } -} diff --git a/src/cmd/go/internal/renameio/umask_test.go b/src/cmd/go/internal/renameio/umask_test.go deleted file mode 100644 index bed45af6ed..0000000000 --- a/src/cmd/go/internal/renameio/umask_test.go +++ /dev/null @@ -1,43 +0,0 @@ -// Copyright 2019 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -//go:build !plan9 && !windows && !js -// +build !plan9,!windows,!js - -package renameio - -import ( - "io/fs" - "os" - "path/filepath" - "syscall" - "testing" -) - -func TestWriteFileModeAppliesUmask(t *testing.T) { - dir, err := os.MkdirTemp("", "renameio") - if err != nil { - t.Fatalf("Failed to create temporary directory: %v", err) - } - defer os.RemoveAll(dir) - - const mode = 0644 - const umask = 0007 - defer syscall.Umask(syscall.Umask(umask)) - - file := filepath.Join(dir, "testWrite") - err = WriteFile(file, []byte("go-build"), mode) - if err != nil { - t.Fatalf("Failed to write file: %v", err) - } - - fi, err := os.Stat(file) - if err != nil { - t.Fatalf("Stat %q (looking for mode %#o): %s", file, mode, err) - } - - if fi.Mode()&fs.ModePerm != 0640 { - t.Errorf("Stat %q: mode %#o want %#o", file, fi.Mode()&fs.ModePerm, 0640) - } -}