clones/go
1
0
Fork 0
mirror of https://github.com/golang/go.git synced 2024-09-21 18:38:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

cmd/go: remove renameio package and its last usage

Browse Source 
The last primary usage of renameio was the WriteFile in
modfetch.WriteDiskCache. Because it's not guaranteed that the fsync in
WriteDiskCache will eliminate file corruption, and it slows down tests
on Macs significantly, inline that last usage, removing the fsync.

Also, remove the uses of renameio.Pattern. The ziphash file is no
longer written to a temporary location before being copied to its
final location, so that usage can just be cut. The remaining use is
for the zipfile . Remove the first because the files are no longer
written using the pattern anyway, so that the pattern variable has no
effect. Replace it with a local pattern variable that is also passed
to os.CreateTemp.

Change-Id: Icf3adabf2a26c37b82afa1d07f821a46b30d69ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/301889
Trust: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
This commit is contained in:
Michael Matloob 2021-03-15 13:59:24 -04:00
parent e726e2a608
commit b7cb92ad12
5 changed files with 45 additions and 302 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
src/cmd/go/internal/modfetch/cache.go
View File

@ -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) {

12
src/cmd/go/internal/modfetch/fetch.go
View File

@ -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
}

88
src/cmd/go/internal/renameio/renameio.go
View File

@ -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
}

161
src/cmd/go/internal/renameio/renameio_test.go
View File

@ -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)
}
}

43
src/cmd/go/internal/renameio/umask_test.go
View File

@ -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)
}
}