diff --git a/changelog/unreleased/issue-3465 b/changelog/unreleased/issue-3465 new file mode 100644 index 000000000..b108461d4 --- /dev/null +++ b/changelog/unreleased/issue-3465 @@ -0,0 +1,10 @@ +Enhancement: Improve handling of temporary files on Windows + +In some cases restic failed to delete temporary files causing the current +command to fail. This has been fixed by ensuring that Windows automatically +deletes the file. In addition, temporary files are only written to disk if +necessary to reduce disk writes. + +https://github.com/restic/restic/issues/3465 +https://github.com/restic/restic/issues/1551 +https://github.com/restic/restic/pull/3610 diff --git a/internal/fs/file_windows.go b/internal/fs/file_windows.go index 8a4d01fb0..56effdc18 100644 --- a/internal/fs/file_windows.go +++ b/internal/fs/file_windows.go @@ -1,10 +1,14 @@ package fs import ( - "io/ioutil" + "math/rand" "os" "path/filepath" + "strconv" "strings" + "time" + + "golang.org/x/sys/windows" ) // fixpath returns an absolute path on windows, so restic can open long file @@ -30,9 +34,43 @@ func fixpath(name string) string { return name } -// TempFile creates a temporary file. +// TempFile creates a temporary file which is marked as delete-on-close func TempFile(dir, prefix string) (f *os.File, err error) { - return ioutil.TempFile(dir, prefix) + // slightly modified implementation of ioutil.TempFile(dir, prefix) to allow us to add + // the FILE_ATTRIBUTE_TEMPORARY | FILE_FLAG_DELETE_ON_CLOSE flags. + // These provide two large benefits: + // FILE_ATTRIBUTE_TEMPORARY tells Windows to keep the file in memory only if possible + // which reduces the amount of unnecessary disk writes. + // FILE_FLAG_DELETE_ON_CLOSE instructs Windows to automatically delete the file once + // all file descriptors are closed. + + if dir == "" { + dir = os.TempDir() + } + + access := uint32(windows.GENERIC_READ | windows.GENERIC_WRITE) + creation := uint32(windows.CREATE_NEW) + share := uint32(0) // prevent other processes from accessing the file + flags := uint32(windows.FILE_ATTRIBUTE_TEMPORARY | windows.FILE_FLAG_DELETE_ON_CLOSE) + + rnd := rand.New(rand.NewSource(time.Now().UnixNano())) + for i := 0; i < 10000; i++ { + randSuffix := strconv.Itoa(int(1e9 + rnd.Intn(1e9)%1e9))[1:] + path := filepath.Join(dir, prefix+randSuffix) + + ptr, err := windows.UTF16PtrFromString(path) + if err != nil { + return nil, err + } + h, err := windows.CreateFile(ptr, access, share, nil, creation, flags, 0) + if os.IsExist(err) { + continue + } + return os.NewFile(uintptr(h), path), err + } + + // Proper error handling is still to do + return nil, os.ErrExist } // Chmod changes the mode of the named file to mode. diff --git a/internal/fs/file_windows_test.go b/internal/fs/file_windows_test.go new file mode 100644 index 000000000..71077709b --- /dev/null +++ b/internal/fs/file_windows_test.go @@ -0,0 +1,35 @@ +package fs_test + +import ( + "errors" + "os" + "testing" + + "github.com/restic/restic/internal/fs" + rtest "github.com/restic/restic/internal/test" +) + +func TestTempFile(t *testing.T) { + // create two temp files at the same time to check that the + // collision avoidance works + f, err := fs.TempFile("", "test") + fn := f.Name() + rtest.OK(t, err) + f2, err := fs.TempFile("", "test") + fn2 := f2.Name() + rtest.OK(t, err) + rtest.Assert(t, fn != fn2, "filenames don't differ %s", fn) + + _, err = os.Stat(fn) + rtest.OK(t, err) + _, err = os.Stat(fn2) + rtest.OK(t, err) + + rtest.OK(t, f.Close()) + rtest.OK(t, f2.Close()) + + _, err = os.Stat(fn) + rtest.Assert(t, errors.Is(err, os.ErrNotExist), "err %s", err) + _, err = os.Stat(fn2) + rtest.Assert(t, errors.Is(err, os.ErrNotExist), "err %s", err) +}