Watcher: list a folder without blocking its removal (restores failing since #100)

internal/fsx/readdirshared.go

@@ -0,0 +1,16 @@
+package fsx
+
+import "os"
+
+// ReadDirShared is os.ReadDir that lets another program delete the folder, or
+// anything in it, while it is being listed.
+//
+// The same default as OpenShared's, for folders: on Windows os.ReadDir opens
+// the folder without FILE_SHARE_DELETE, and while it is open the folder cannot
+// be removed — "being used by another process". The watcher lists a folder
+// whenever something inside it changes (checkFolders), so it was listing the
+// folders a restore was emptying, and the restore failed; a game deleting a
+// folder of its own saves at that moment fails the same way.
+func ReadDirShared(path string) ([]os.DirEntry, error) {
+	return readDirShared(path)
+}

internal/fsx/readdirshared_other.go

@@ -0,0 +1,10 @@
+//go:build !windows
+
+package fsx
+
+import "os"
+
+// On Unix an open folder never blocks removing it or anything in it.
+func readDirShared(path string) ([]os.DirEntry, error) {
+	return os.ReadDir(path)
+}

internal/fsx/readdirshared_windows.go

@@ -0,0 +1,39 @@
+package fsx
+
+import (
+	"os"
+	"syscall"
+)
+
+// readDirShared lists the folder through a handle opened with
+// FILE_SHARE_DELETE (FILE_FLAG_BACKUP_SEMANTICS is what opens a folder). Any
+// failure to open it so falls back to os.ReadDir, as openShared does, and for
+// the same reason: os.ReadDir's path handling is not to be second-guessed.
+func readDirShared(path string) ([]os.DirEntry, error) {
+	f, err := openDirShared(path)
+	if err != nil {
+		return os.ReadDir(path)
+	}
+	defer f.Close()
+	return f.ReadDir(-1)
+}
+
+func openDirShared(path string) (*os.File, error) {
+	p, err := syscall.UTF16PtrFromString(path)
+	if err != nil {
+		return nil, err
+	}
+	h, err := syscall.CreateFile(
+		p,
+		syscall.GENERIC_READ,
+		syscall.FILE_SHARE_READ|syscall.FILE_SHARE_WRITE|syscall.FILE_SHARE_DELETE,
+		nil,
+		syscall.OPEN_EXISTING,
+		syscall.FILE_FLAG_BACKUP_SEMANTICS,
+		0,
+	)
+	if err != nil {
+		return nil, err
+	}
+	return os.NewFile(uintptr(h), path), nil
+}

internal/fsx/readdirshared_windows_test.go

@@ -0,0 +1,67 @@
+package fsx
+
+import (
+	"os"
+	"path/filepath"
+	"sort"
+	"testing"
+)
+
+func folderWithSaves(t *testing.T) string {
+	t.Helper()
+	dir := filepath.Join(t.TempDir(), "slot1")
+	if err := os.MkdirAll(filepath.Join(dir, "sub"), 0o777); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(dir, "a.sav"), []byte("x"), 0o666); err != nil {
+		t.Fatal(err)
+	}
+	return dir
+}
+
+// A folder being listed can still be removed: what a restore emptying the save
+// folder, or a game deleting a folder of its own, does while the watcher looks
+// inside it. The same with os.Open is refused, which is why this exists.
+func TestReadDirShared_TheFolderCanBeRemovedWhileListed(t *testing.T) {
+	dir := folderWithSaves(t)
+	f, err := openDirShared(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := os.RemoveAll(dir); err != nil {
+		t.Errorf("a folder being listed could not be removed: %v", err)
+	}
+	f.Close()
+
+	dir = folderWithSaves(t)
+	plain, err := os.Open(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer plain.Close()
+	if err := os.RemoveAll(dir); err == nil {
+		t.Log("NOTE: a folder held open by os.Open can now be removed on this Windows build")
+	}
+}
+
+func TestReadDirShared_ListsAsReadDirDoes(t *testing.T) {
+	dir := folderWithSaves(t)
+	got, err := ReadDirShared(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var names []string
+	for _, e := range got {
+		names = append(names, e.Name())
+		if (e.Name() == "sub") != e.IsDir() {
+			t.Errorf("%s: IsDir %v", e.Name(), e.IsDir())
+		}
+	}
+	sort.Strings(names)
+	if len(names) != 2 || names[0] != "a.sav" || names[1] != "sub" {
+		t.Errorf("listed %v, want [a.sav sub]", names)
+	}
+	if _, err := ReadDirShared(filepath.Join(dir, "missing")); !os.IsNotExist(err) {
+		t.Errorf("a missing folder: %v, want not-exist", err)
+	}
+}

internal/watcher/watcher.go

@@ -31,6 +31,7 @@ import (
 
 	"github.com/fsnotify/fsnotify"
 	"github.com/opensave/opensave/internal/delta"
+	"github.com/opensave/opensave/internal/fsx"
 	"github.com/opensave/opensave/internal/ignore"
 	"github.com/opensave/opensave/internal/logging"
 	"github.com/opensave/opensave/internal/owntouch"
@@ -1129,7 +1130,10 @@ func (e *Engine) registerFolders(ctx context.Context, gw *gameWatch) {
 // looked up once per check, only when there are subfolders to look up. A
 // burst of Writes to one folder is one check (queueCheck).
 func (e *Engine) checkFolders(ctx context.Context, gw *gameWatch, dir string) bool {
-	entries, err := os.ReadDir(dir)
+	// Listed so that it can still be removed meanwhile: the Write that brought
+	// this here may be a restore, or the game, emptying the folder
+	// (fsx.ReadDirShared).
+	entries, err := fsx.ReadDirShared(dir)
 	if err != nil {
 		return false // gone again, or unreadable: its own events will tell
 	}