Merge pull request #3610 from MichaelEischer/windows-temp-files

Improve handling of temporary files on windows

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

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.

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