diff --git a/magpie/docs/subsystems/wefto-storage.md b/magpie/docs/subsystems/wefto-storage.md index ceb63c532d..589b5a6a2f 100644 --- a/magpie/docs/subsystems/wefto-storage.md +++ b/magpie/docs/subsystems/wefto-storage.md @@ -22,6 +22,8 @@ Provider records contain credential references and previews; API keys remain in ## Shared file replacement +Windows [`steady` readers and publishers](../../internal/steady/guard_windows.go) coordinate each metadata lookup, content read and rename through a named kernel mutex for the normalized absolute pathname. Relative, case-varied and extended Win32 spellings share that name; the hash keeps the pathname out of the kernel object name. The mutex spans processes and Windows sessions on this computer, uses the creator's default access control, and creates no files during a read. Each acquisition queues for at most the existing 10 ms pause so continuous publishers cannot win every immediate poll. A busy mutex returns a sharing error to the existing bounded retry counts; cancellation is checked between attempts. Ownership stays on one OS thread through release, and an abandoned mutex is acquired normally after its owner exits. Different pathnames remain independent. Files reached through distinct link or short-name aliases, and older or external writers that do not use this protocol, retain the bounded I/O retries rather than this coordination guarantee. + [`steady.Rename` and `WriteFileContext`](../../internal/steady/steady.go) publish staged state files beside their destination. On Windows these sibling replacements use [`os.Root.Rename`](../../internal/steady/rename_windows.go), which requests handle-based POSIX replacement and owns the filesystem fallback. On supporting filesystems such as NTFS, a reader that permits deletion can finish reading the previous generation while new readers open the complete replacement. A reader that denies replacement is still waited out by the existing bounded retry. Moves between different directory paths retain `os.Rename` behavior; the staged file is flushed before publication and a failed write removes its staging file. [`ReadFile` / `ReadThere`](../../internal/steady/read_windows.go) open Windows files with read, write and delete sharing, so reads of the previous generation cooperate with that replacement. Each read owns and closes one handle; ordinary Win32 resolution still follows links outside the parent, and long absolute and relative paths retain extended-path support. Sharing flags do not grant read access: a handle that denies readers still produces a read-sharing error after the bounded retry. diff --git a/magpie/internal/steady/guard_windows.go b/magpie/internal/steady/guard_windows.go new file mode 100644 index 0000000000..9fd664f29c --- /dev/null +++ b/magpie/internal/steady/guard_windows.go @@ -0,0 +1,74 @@ +package steady + +import ( + "crypto/sha256" + "errors" + "fmt" + "os" + "path/filepath" + "runtime" + "strings" + "time" + + winapi "golang.org/x/sys/windows" +) + +// A successful Windows replacement can still hide the name while another +// thread is publishing it. Coordinate each lookup/read and publication by +// pathname across processes, without creating a lock file during a read. +// A busy mutex is a sharing error, handled by the existing bounded retries. +func lockPath(path string) (func(), error) { + handle, err := openPathMutex(path) + if err != nil { + return nil, err + } + // Windows mutex ownership belongs to an OS thread, not a goroutine. + runtime.LockOSThread() + // Queue briefly in the kernel: polling a busy mutex lets continuous + // publishers starve a reader even though each replacement is short. + state, err := winapi.WaitForSingleObject(handle, uint32(pause/time.Millisecond)) + if err == nil && (state == winapi.WAIT_OBJECT_0 || state == winapi.WAIT_ABANDONED) { + return func() { + winapi.ReleaseMutex(handle) + winapi.CloseHandle(handle) + runtime.UnlockOSThread() + }, nil + } + winapi.CloseHandle(handle) + runtime.UnlockOSThread() + if err == nil { + err = winapi.ERROR_SHARING_VIOLATION + } + return nil, &os.PathError{Op: "lock", Path: path, Err: err} +} + +func openPathMutex(path string) (winapi.Handle, error) { + abs, err := filepath.Abs(path) + if err != nil { + return 0, err + } + // Extended and ordinary Win32 spellings must name the same mutex. + abs = strings.ToUpper(abs) + if strings.HasPrefix(abs, `\\?\UNC\`) { + abs = `\\` + abs[len(`\\?\UNC\`):] + } else { + abs = strings.TrimPrefix(abs, `\\?\`) + } + key := sha256.Sum256([]byte(filepath.Clean(abs))) + name, err := winapi.UTF16PtrFromString(fmt.Sprintf(`Global\steady-file-%x`, key)) + if err != nil { + return 0, err + } + handle, err := winapi.CreateMutexEx(nil, name, 0, winapi.SYNCHRONIZE|winapi.MUTEX_MODIFY_STATE) + // x/sys preserves ERROR_ALREADY_EXISTS alongside a valid shared handle. + if handle != 0 && errors.Is(err, winapi.ERROR_ALREADY_EXISTS) { + err = nil + } + if err != nil { + if handle != 0 { + winapi.CloseHandle(handle) + } + return 0, &os.PathError{Op: "lock", Path: path, Err: err} + } + return handle, nil +} diff --git a/magpie/internal/steady/guard_windows_test.go b/magpie/internal/steady/guard_windows_test.go new file mode 100644 index 0000000000..ed19dca92e --- /dev/null +++ b/magpie/internal/steady/guard_windows_test.go @@ -0,0 +1,163 @@ +package steady + +import ( + "bufio" + "context" + "errors" + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + winapi "golang.org/x/sys/windows" +) + +// The reader and publisher must coordinate between processes, including +// relative, case-varied and extended spellings of the same pathname. +func TestReplacementGuardAcrossProcesses(t *testing.T) { + t.Chdir(t.TempDir()) + workingDirectory, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + for _, operation := range []string{"read", "stat", "rename", "write"} { + t.Run(operation, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "Accounts.json") + if err := os.WriteFile(path, []byte("old accounts"), 0o600); err != nil { + t.Fatal(err) + } + staged := path + ".tmp" + if err := os.WriteFile(staged, []byte("new accounts"), 0o600); err != nil { + t.Fatal(err) + } + relative, err := filepath.Rel(workingDirectory, path) + if err != nil { + t.Fatal(err) + } + apply := func() error { + switch operation { + case "read": + _, err := ReadFile(strings.ToUpper(path)) + return err + case "stat": + _, err := Stat(path) + return err + case "rename": + return Rename(staged, `\\?\`+path) + default: + return WriteFile(relative, []byte("new accounts"), 0o600) + } + } + release := holdPathInProcess(t, path, false) + if err := apply(); !errors.Is(err, winapi.ERROR_SHARING_VIOLATION) { + t.Errorf("%s crossed another process's publication: %v", operation, err) + } + if body, err := os.ReadFile(path); err != nil || string(body) != "old accounts" { + t.Errorf("a blocked publication changed the generation: %q, %v", body, err) + } + other := path + ".other" + if err := os.WriteFile(other, []byte("independent"), 0o600); err != nil { + t.Fatal(err) + } + if body, err := ReadFile(other); err != nil || string(body) != "independent" { + t.Errorf("an unrelated pathname was blocked: %q, %v", body, err) + } + release() + if err := apply(); err != nil { + t.Fatalf("%s after the publisher released: %v", operation, err) + } + }) + } +} + +func TestReplacementGuardRecoversAfterProcessExit(t *testing.T) { + path := filepath.Join(t.TempDir(), "accounts.json") + if err := os.WriteFile(path, []byte("accounts"), 0o600); err != nil { + t.Fatal(err) + } + release := holdPathInProcess(t, path, true) + // Keep the kernel object alive so the next wait observes abandonment, + // rather than creating a new mutex after the old owner's exit. + handle, err := openPathMutex(path) + if err != nil { + t.Fatal(err) + } + defer winapi.CloseHandle(handle) + release() + if body, err := ReadFile(path); err != nil || string(body) != "accounts" { + t.Fatalf("read after the owning process exited: %q, %v", body, err) + } +} + +func holdPathInProcess(t *testing.T, path string, abandon bool) func() { + t.Helper() + ctx, cancel := context.WithTimeout(t.Context(), 15*time.Second) + mode := "--steady-guard-release" + if abandon { + mode = "--steady-guard-abandon" + } + cmd := exec.CommandContext(ctx, os.Args[0], "-test.run=^TestReplacementGuardOtherProcess$", "--", mode, path) + input, err := cmd.StdinPipe() + if err != nil { + cancel() + t.Fatal(err) + } + output, err := cmd.StdoutPipe() + if err != nil { + cancel() + t.Fatal(err) + } + cmd.Stderr = os.Stderr + if err := cmd.Start(); err != nil { + cancel() + t.Fatal(err) + } + release := sync.OnceFunc(func() { + _, _ = input.Write([]byte("x")) + input.Close() + if err := cmd.Wait(); err != nil { + t.Errorf("publication helper: %v", err) + } + cancel() + }) + t.Cleanup(release) + ready := make(chan error, 1) + go func() { + line, err := bufio.NewReader(output).ReadString('\n') + if err == nil && strings.TrimSpace(line) != "ready" { + err = fmt.Errorf("unexpected publication helper output: %q", line) + } + ready <- err + }() + select { + case err := <-ready: + if err != nil { + t.Fatal(err) + } + case <-ctx.Done(): + t.Fatal(ctx.Err()) + } + return release +} + +func TestReplacementGuardOtherProcess(t *testing.T) { + args := os.Args + if len(args) < 3 || !strings.HasPrefix(args[len(args)-2], "--steady-guard-") { + return + } + unlock, err := lockPath(args[len(args)-1]) + if err != nil { + t.Fatal(err) + } + fmt.Fprintln(os.Stdout, "ready") + _, _ = io.CopyN(io.Discard, os.Stdin, 1) + if args[len(args)-2] == "--steady-guard-abandon" { + os.Exit(0) + } + unlock() +} diff --git a/magpie/internal/steady/read_windows.go b/magpie/internal/steady/read_windows.go index 6eed555a11..2a65cface1 100644 --- a/magpie/internal/steady/read_windows.go +++ b/magpie/internal/steady/read_windows.go @@ -51,6 +51,11 @@ func openSharedFile(path string, access uint32) (*os.File, error) { } func readOnce(path string) ([]byte, error) { + unlock, err := lockPath(path) + if err != nil { + return nil, err + } + defer unlock() f, err := openReadFile(path) if err != nil { return nil, err diff --git a/magpie/internal/steady/rename_windows.go b/magpie/internal/steady/rename_windows.go index 366eadaae9..a3c6e01a11 100644 --- a/magpie/internal/steady/rename_windows.go +++ b/magpie/internal/steady/rename_windows.go @@ -6,6 +6,15 @@ import ( ) func rename(oldpath, newpath string) error { + unlock, err := lockPath(newpath) + if err != nil { + return &os.LinkError{Op: "rename", Old: oldpath, New: newpath, Err: err} + } + defer unlock() + return renameUnlocked(oldpath, newpath) +} + +func renameUnlocked(oldpath, newpath string) error { if oldpath == "" || newpath == "" || os.IsPathSeparator(oldpath[len(oldpath)-1]) || os.IsPathSeparator(newpath[len(newpath)-1]) { return os.Rename(oldpath, newpath) } diff --git a/magpie/internal/steady/stat_windows.go b/magpie/internal/steady/stat_windows.go index a61b3e8836..0cf9881290 100644 --- a/magpie/internal/steady/stat_windows.go +++ b/magpie/internal/steady/stat_windows.go @@ -7,6 +7,11 @@ import ( ) func statOnce(path string) (os.FileInfo, error) { + unlock, err := lockPath(path) + if err != nil { + return nil, err + } + defer unlock() // Query an opened generation instead of Win32's path-attribute lookup, // which can report a replaced name absent. Metadata needs no read-data // access and must not deny another publisher's delete access.