refactor(general): leverage os.CreateTemp (#4513)

* use `os.CreateTemp` in `tempfile.CreateAutoDelete`
* refactor `TestTempFile`: add `VerifyTempfile` test helper
* add test for `createUnixFallback`
* rename function to `tempfile.CreateAutoDelete`
* remove the `dir` parameter to `tempfile.CreateAutoDelete`,
  only an empty string was passed, apart from test cases.
* guard against panic in TestShadowCopy

internal/bigmap/bigmap_internal.go

@@ -344,7 +344,7 @@ func (m *internalMap) newMemoryMappedSegment(ctx context.Context) (mmap.MMap, er
 
 // +checklocks:m.mu
 func (m *internalMap) maybeCreateMappedFile(ctx context.Context) (*os.File, error) {
-	f, err := tempfile.Create("")
+	f, err := tempfile.CreateAutoDelete()
 	if err != nil {
 		return nil, errors.Wrap(err, "unable to create memory-mapped file")
 	}

internal/tempfile/tempfile.go

@@ -1,13 +1,3 @@
 // Package tempfile provides a cross-platform abstraction for creating private
 // read-write temporary files which are automatically deleted when closed.
 package tempfile
-
-import "os"
-
-func tempDirOr(dir string) string {
-	if dir != "" {
-		return dir
-	}
-
-	return os.TempDir()
-}

internal/tempfile/tempfile_linux.go

@@ -8,9 +8,11 @@
 	"golang.org/x/sys/unix"
 )
 
-// Create creates a temporary file that will be automatically deleted on close.
-func Create(dir string) (*os.File, error) {
-	dir = tempDirOr(dir)
+const permissions = 0o600
+
+// CreateAutoDelete creates a temporary file that will be automatically deleted on close.
+func CreateAutoDelete() (*os.File, error) {
+	dir := os.TempDir()
 
 	// on reasonably modern Linux (3.11 and above) O_TMPFILE is supported,
 	// which creates invisible, unlinked file in a given directory.
@@ -20,7 +22,7 @@ func Create(dir string) (*os.File, error) {
 	}
 
 	if errors.Is(err, syscall.EISDIR) || errors.Is(err, syscall.EOPNOTSUPP) {
-		return createUnixFallback(dir)
+		return createUnixFallback()
 	}
 
 	return nil, &os.PathError{

internal/tempfile/tempfile_linux_fallback_test.go

@@ -0,0 +1,16 @@
+//go:build linux
+// +build linux
+
+package tempfile
+
+import (
+	"testing"
+)
+
+// Explicitly test the create fallback function.
+// This test only applies to Linux to explicitly test the fallback function.
+// In other unix platforms the fallback is the default implementation, so it
+// is already tested in the tests for the Create function.
+func TestCreateFallback(t *testing.T) {
+	VerifyTempfile(t, createUnixFallback)
+}

internal/tempfile/tempfile_test.go

@@ -1,47 +1,11 @@
 package tempfile_test
 
 import (
-	"io"
-	"os"
 	"testing"
 
-	"github.com/stretchr/testify/require"
-
 	"github.com/kopia/kopia/internal/tempfile"
 )
 
 func TestTempFile(t *testing.T) {
-	td := t.TempDir()
-
-	f, err := tempfile.Create(td)
-	require.NoError(t, err)
-
-	n, err := f.WriteString("hello")
-	require.NoError(t, err)
-	require.Equal(t, 5, n)
-
-	off, err := f.Seek(1, io.SeekStart)
-	require.Equal(t, int64(1), off)
-	require.NoError(t, err)
-
-	buf := make([]byte, 4)
-	n2, err := f.Read(buf)
-	require.NoError(t, err)
-	require.Equal(t, 4, n2)
-	require.Equal(t, []byte("ello"), buf)
-
-	f.Close()
-
-	files, err := os.ReadDir(td)
-	require.NoError(t, err)
-	require.Empty(t, files)
-}
-
-func TestCreateSucceedsWhenDirIsNotSpecified(t *testing.T) {
-	f, err := tempfile.Create("")
-
-	require.NoError(t, err)
-
-	err = f.Close()
-	require.NoError(t, err)
+	tempfile.VerifyTempfile(t, tempfile.CreateAutoDelete)
 }

internal/tempfile/tempfile_unix_fallback.go

@@ -5,25 +5,19 @@
 
 import (
 	"os"
-	"path/filepath"
 
-	"github.com/google/uuid"
 	"github.com/pkg/errors"
 )
 
-const permissions = 0o600
-
 // createUnixFallback creates a temporary file that does not need to be removed on close.
-func createUnixFallback(dir string) (*os.File, error) {
-	fullPath := filepath.Join(tempDirOr(dir), uuid.NewString())
-
-	f, err := os.OpenFile(fullPath, os.O_CREATE|os.O_EXCL|os.O_RDWR, permissions) //nolint:gosec
+func createUnixFallback() (*os.File, error) {
+	f, err := os.CreateTemp("", "kt-")
 	if err != nil {
 		return nil, err //nolint:wrapcheck
 	}
 
 	// immediately remove/unlink the file while we keep the handle open.
-	if derr := os.Remove(fullPath); derr != nil {
+	if derr := os.Remove(f.Name()); derr != nil {
 		f.Close() //nolint:errcheck
 		return nil, errors.Wrap(derr, "unable to unlink temporary file")
 	}

internal/tempfile/tempfile_unix_nonlinux.go

@@ -7,7 +7,7 @@
 	"os"
 )
 
-// Create creates a temporary file that does not need to be removed on close.
-func Create(dir string) (*os.File, error) {
-	return createUnixFallback(dir)
+// CreateAutoDelete creates a temporary file that does not need to be explicitly removed on close.
+func CreateAutoDelete() (*os.File, error) {
+	return createUnixFallback()
 }

internal/tempfile/tempfile_verify_test.go

@@ -0,0 +1,48 @@
+package tempfile
+
+import (
+	"io"
+	"os"
+	"runtime"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func VerifyTempfile(t *testing.T, create func() (*os.File, error)) {
+	t.Helper()
+
+	f, err := create()
+	require.NoError(t, err)
+
+	n, err := f.WriteString("hello")
+	require.NoError(t, err)
+	require.Equal(t, 5, n)
+
+	off, err := f.Seek(1, io.SeekStart)
+	require.Equal(t, int64(1), off)
+	require.NoError(t, err)
+
+	buf := make([]byte, 4)
+	n2, err := f.Read(buf)
+	require.NoError(t, err)
+	require.Equal(t, 4, n2)
+	require.Equal(t, []byte("ello"), buf)
+
+	f.Close()
+
+	if n := f.Name(); n != "" {
+		var perr *os.PathError
+
+		// there should be no directory entry for this file
+		_, err := os.Stat(n)
+
+		require.Error(t, err)
+		require.ErrorAs(t, err, &perr)
+		if runtime.GOOS == "windows" {
+			require.ErrorContains(t, err, "The system cannot find the file specified")
+		} else {
+			require.ErrorContains(t, err, "no such file or directory")
+		}
+	}
+}

internal/tempfile/tempfile_windows.go

@@ -9,9 +9,9 @@
 	"golang.org/x/sys/windows"
 )
 
-// Create creates a temporary file that will be automatically deleted on close.
-func Create(dir string) (*os.File, error) {
-	fullpath := filepath.Join(tempDirOr(dir), uuid.NewString())
+// CreateAutoDelete creates a temporary file that will be automatically deleted on close.
+func CreateAutoDelete() (*os.File, error) {
+	fullpath := filepath.Join(os.TempDir(), uuid.NewString())
 
 	fname, err := syscall.UTF16PtrFromString(fullpath)
 	if err != nil {

tests/os_snapshot_test/os_snapshot_windows_test.go

@@ -2,13 +2,16 @@
 
 import (
 	"os"
+	"path/filepath"
+	"syscall"
 	"testing"
 
+	"github.com/google/uuid"
 	"github.com/mxk/go-vss"
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/sys/windows"
 
-	"github.com/kopia/kopia/internal/tempfile"
 	"github.com/kopia/kopia/internal/testutil"
 	"github.com/kopia/kopia/tests/clitestutil"
 	"github.com/kopia/kopia/tests/testenv"
@@ -21,21 +24,19 @@ func TestShadowCopy(t *testing.T) {
 	}
 
 	runner := testenv.NewExeRunnerWithBinary(t, kopiaExe)
-
 	e := testenv.NewCLITest(t, testenv.RepoFormatNotImportant, runner)
-	e.RunAndExpectSuccess(t, "repo", "create", "filesystem", "--path", e.RepoDir)
 
+	e.RunAndExpectSuccess(t, "repo", "create", "filesystem", "--path", e.RepoDir)
+	e.RunAndExpectSuccess(t, "policy", "set", "--global", "--enable-volume-shadow-copy=when-available")
+
+	// create a file that cannot be accessed and then attempt to create a snapshot
 	root := testutil.TempDirectory(t)
-	f, err := tempfile.Create(root)
-	require.NoError(t, err)
-	_, err = f.WriteString("locked file\n")
+	f := createAutoDelete(t, root)
+	_, err := f.WriteString("locked file\n")
+
 	require.NoError(t, err)
 	require.NoError(t, f.Sync())
 
-	defer f.Close()
-
-	e.RunAndExpectSuccess(t, "policy", "set", "--global", "--enable-volume-shadow-copy=when-available")
-
 	_, err = vss.Get("{00000000-0000-0000-0000-000000000000}")
 
 	isAdmin := !errors.Is(err, os.ErrPermission)
@@ -43,7 +44,7 @@ func TestShadowCopy(t *testing.T) {
 		t.Log("Running as admin, expecting snapshot creation to succeed")
 		e.RunAndExpectSuccess(t, "snap", "create", root)
 	} else {
-		t.Log("Not running as admin, expecting snapshot creation to fail")
+		t.Log("Not running as admin, expecting snapshot creation to fail because it cannot access the file that is in use")
 		e.RunAndExpectFailure(t, "snap", "create", root)
 	}
 
@@ -54,11 +55,42 @@ func TestShadowCopy(t *testing.T) {
 
 	oid := sources[0].Snapshots[0].ObjectID
 	entries := clitestutil.ListDirectory(t, e, oid)
+	t.Log("sources[0].Snapshots[0].ObjectID entries:", entries)
 
 	if isAdmin {
+		require.NotEmpty(t, entries)
 		lines := e.RunAndExpectSuccess(t, "show", entries[0].ObjectID)
 		require.Equal(t, []string{"locked file"}, lines)
 	} else {
 		require.Empty(t, entries)
 	}
 }
+
+func createAutoDelete(t *testing.T, dir string) *os.File {
+	t.Helper()
+
+	fullpath := filepath.Join(dir, uuid.NewString())
+
+	fname, err := syscall.UTF16PtrFromString(fullpath)
+	require.NoError(t, err, "constructing file name UTF16Ptr")
+
+	// This call creates a file that's automatically deleted on close.
+	h, err := syscall.CreateFile(
+		fname,
+		windows.GENERIC_READ|windows.GENERIC_WRITE,
+		0,
+		nil,
+		syscall.OPEN_ALWAYS,
+		uint32(windows.FILE_FLAG_DELETE_ON_CLOSE),
+		0)
+
+	require.NoError(t, err, "creating file")
+
+	f := os.NewFile(uintptr(h), fullpath)
+
+	t.Cleanup(func() {
+		f.Close()
+	})
+
+	return f
+}