diff --git a/internal/fsx/readdirshared.go b/internal/fsx/readdirshared.go new file mode 100644 index 00000000..3f9dcf60 --- /dev/null +++ b/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) +} diff --git a/internal/fsx/readdirshared_other.go b/internal/fsx/readdirshared_other.go new file mode 100644 index 00000000..b38aea0f --- /dev/null +++ b/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) +} diff --git a/internal/fsx/readdirshared_windows.go b/internal/fsx/readdirshared_windows.go new file mode 100644 index 00000000..43942b5b --- /dev/null +++ b/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 +} diff --git a/internal/fsx/readdirshared_windows_test.go b/internal/fsx/readdirshared_windows_test.go new file mode 100644 index 00000000..17f9a976 --- /dev/null +++ b/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) + } +} diff --git a/internal/watcher/watcher.go b/internal/watcher/watcher.go index a5477f3f..08d10258 100644 --- a/internal/watcher/watcher.go +++ b/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 }