From 5996286a89975217c805c7c85e7d59ea9c5048ea Mon Sep 17 00:00:00 2001 From: patterniha <71074308+patterniha@users.noreply.github.com> Date: Mon, 2 Nov 2026 12:23:24 +0330 Subject: [PATCH] XDRIVE transport: Retry "local" Get on Windows sharing violations On Windows, os.Rename (MoveFileEx) opens the file with DELETE access, renames it, then closes that handle. For a moment the new name is already listed while the handle is still open, and os.ReadFile, which shares only reading and writing, fails with ERROR_SHARING_VIOLATION. The WAL reader lists a segment as soon as Put renames it into place, and any Get error other than "not found" ends the stream: xdrive_test.go:143: ReadFull: transport/internet/xdrive: failed to read streams//c2s/000000001.seg > open ...\000000001.seg: The process cannot access the file because it is being used by another process. Measured on Windows 11 (Defender real-time protection on, 22 logical CPUs) with a reader that keeps opening the new name during the rename: 98.6% of 5,001 renames hit the error, every failed open started before MoveFileEx returned, and none failed with FILE_SHARE_DELETE added to the open. The window is about 0.3 ms when idle, but 10 ms median and up to 124 ms with all CPUs busy, which is why it shows up in full `go test ./...` runs. Under such load (22 busy-loop processes) the unfixed TestMultiSegmentTransfer, TestRoundTrip, TestInterleaved and TestCloseEOF failed 4 of 60 runs; with this change, 0 of 120. Get now retries os.ReadFile on ERROR_SHARING_VIOLATION for up to 2 seconds, as cmd/internal/robustio does, backing off from 1 ms to 100 ms and returning early once ctx is done. Other platforms still call os.ReadFile once. TestLocalSharingViolation holds a segment open with DELETE access, as MoveFileEx does, and checks that Get waits for it. Opening with FILE_SHARE_DELETE would avoid the conflict, but os.Open cannot set the share mode, and a raw CreateFile would lose its long path handling. Put's rename is unchanged: no rename failed in about 63,000 renames during these measurements, idle or under load. Co-Authored-By: Claude Opus 5.5 --- transport/internet/xdrive/local.go | 2 +- transport/internet/xdrive/local_other.go | 12 ++++ transport/internet/xdrive/local_windows.go | 30 ++++++++++ .../internet/xdrive/local_windows_test.go | 57 +++++++++++++++++++ 4 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 transport/internet/xdrive/local_other.go create mode 100644 transport/internet/xdrive/local_windows.go create mode 100644 transport/internet/xdrive/local_windows_test.go diff --git a/transport/internet/xdrive/local.go b/transport/internet/xdrive/local.go index 28757a10e..6527600ef 100644 --- a/transport/internet/xdrive/local.go +++ b/transport/internet/xdrive/local.go @@ -72,7 +72,7 @@ func (s *localStorage) Get(ctx context.Context, name string) ([]byte, error) { if err != nil { return nil, err } - data, err := os.ReadFile(full) + data, err := readFile(ctx, full) if err != nil { if os.IsNotExist(err) { return nil, errNotFound diff --git a/transport/internet/xdrive/local_other.go b/transport/internet/xdrive/local_other.go new file mode 100644 index 000000000..2b0017fac --- /dev/null +++ b/transport/internet/xdrive/local_other.go @@ -0,0 +1,12 @@ +//go:build !windows + +package xdrive + +import ( + "context" + "os" +) + +func readFile(_ context.Context, name string) ([]byte, error) { + return os.ReadFile(name) +} diff --git a/transport/internet/xdrive/local_windows.go b/transport/internet/xdrive/local_windows.go new file mode 100644 index 000000000..34fc5695c --- /dev/null +++ b/transport/internet/xdrive/local_windows.go @@ -0,0 +1,30 @@ +package xdrive + +import ( + "context" + "errors" + "os" + "time" + + "golang.org/x/sys/windows" +) + +// readFile retries os.ReadFile for up to 2 seconds on sharing violations: +// os.Rename (MoveFileEx) in Put holds the file with DELETE access, which +// os.ReadFile does not share, until a moment after its new name appears in List +func readFile(ctx context.Context, name string) ([]byte, error) { + deadline := time.Now().Add(2 * time.Second) + delay := time.Millisecond + for { + data, err := os.ReadFile(name) + if !errors.Is(err, windows.ERROR_SHARING_VIOLATION) || time.Now().Add(delay).After(deadline) { + return data, err + } + select { + case <-ctx.Done(): + return nil, err + case <-time.After(delay): + } + delay = min(2*delay, 100*time.Millisecond) + } +} diff --git a/transport/internet/xdrive/local_windows_test.go b/transport/internet/xdrive/local_windows_test.go new file mode 100644 index 000000000..6e01f1313 --- /dev/null +++ b/transport/internet/xdrive/local_windows_test.go @@ -0,0 +1,57 @@ +package xdrive + +import ( + "context" + "errors" + "sync" + "testing" + "time" + + "golang.org/x/sys/windows" +) + +func TestLocalSharingViolation(t *testing.T) { + storage, err := newLocalStorage(t.TempDir()) + if err != nil { + t.Fatalf("newLocalStorage: %v", err) + } + const name = "streams/session/c2s/000000000.seg" + if err := storage.Put(context.Background(), name, []byte("segment")); err != nil { + t.Fatalf("Put: %v", err) + } + full, err := storage.resolve(name) + if err != nil { + t.Fatalf("resolve: %v", err) + } + path, err := windows.UTF16PtrFromString(full) + if err != nil { + t.Fatalf("UTF16PtrFromString: %v", err) + } + + // os.Rename holds the file like this until a moment after its new name + // appears + handle, err := windows.CreateFile(path, windows.DELETE, + windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE, + nil, windows.OPEN_EXISTING, 0, 0) + if err != nil { + t.Fatalf("CreateFile: %v", err) + } + var once sync.Once + release := func() { once.Do(func() { windows.CloseHandle(handle) }) } + defer release() + + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + if _, err := storage.Get(ctx, name); !errors.Is(err, windows.ERROR_SHARING_VIOLATION) { + t.Fatalf("Get returned %v while the file is held, want a sharing violation", err) + } + + time.AfterFunc(100*time.Millisecond, release) + data, err := storage.Get(context.Background(), name) + if err != nil { + t.Fatalf("Get: %v", err) + } + if string(data) != "segment" { + t.Fatalf("Get returned %q, want %q", data, "segment") + } +}