mirror of
https://github.com/XTLS/Xray-core.git
synced 2026-11-03 08:57:00 +00:00
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/<id>/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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
7c8dbcb4bb
commit
5996286a89
@@ -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
|
||||
|
||||
@@ -0,0 +1,12 @@
|
||||
//go:build !windows
|
||||
|
||||
package xdrive
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
)
|
||||
|
||||
func readFile(_ context.Context, name string) ([]byte, error) {
|
||||
return os.ReadFile(name)
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user