From 42f0963bf907daacb6cb718ef14a64a591fab3a0 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Wed, 4 Apr 2018 14:24:04 +0100 Subject: [PATCH] local: retry remove on Windows sharing violation error #2202 Before this change asynchronous closes in cmount could cause sharing violations under Windows on Remove which manifest themselves frequently as test failures. This change lets the Remove be retried on a sharing violation under Windows. --- backend/local/local.go | 2 +- backend/local/remove_other.go | 10 +++++++ backend/local/remove_test.go | 50 +++++++++++++++++++++++++++++++++ backend/local/remove_windows.go | 38 +++++++++++++++++++++++++ 4 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 backend/local/remove_other.go create mode 100644 backend/local/remove_test.go create mode 100644 backend/local/remove_windows.go diff --git a/backend/local/local.go b/backend/local/local.go index 935dbecd3..156c2ce26 100644 --- a/backend/local/local.go +++ b/backend/local/local.go @@ -837,7 +837,7 @@ func (o *Object) lstat() error { // Remove an object func (o *Object) Remove() error { - return os.Remove(o.path) + return remove(o.path) } // Return the directory and file from an OS path. Assumes diff --git a/backend/local/remove_other.go b/backend/local/remove_other.go new file mode 100644 index 000000000..760e2cf3b --- /dev/null +++ b/backend/local/remove_other.go @@ -0,0 +1,10 @@ +//+build !windows + +package local + +import "os" + +// Removes name, retrying on a sharing violation +func remove(name string) error { + return os.Remove(name) +} diff --git a/backend/local/remove_test.go b/backend/local/remove_test.go new file mode 100644 index 000000000..b2e34cdc8 --- /dev/null +++ b/backend/local/remove_test.go @@ -0,0 +1,50 @@ +package local + +import ( + "io/ioutil" + "os" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Check we can remove an open file +func TestRemove(t *testing.T) { + fd, err := ioutil.TempFile("", "rclone-remove-test") + require.NoError(t, err) + name := fd.Name() + defer func() { + _ = os.Remove(name) + }() + + exists := func() bool { + _, err := os.Stat(name) + if err == nil { + return true + } else if os.IsNotExist(err) { + return false + } + require.NoError(t, err) + return false + } + + assert.True(t, exists()) + // close the file in the background + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + time.Sleep(250 * time.Millisecond) + require.NoError(t, fd.Close()) + }() + // delete the open file + err = remove(name) + require.NoError(t, err) + // check it no longer exists + assert.False(t, exists()) + // wait for background close + wg.Wait() +} diff --git a/backend/local/remove_windows.go b/backend/local/remove_windows.go new file mode 100644 index 000000000..bf28b2cf3 --- /dev/null +++ b/backend/local/remove_windows.go @@ -0,0 +1,38 @@ +//+build windows + +package local + +import ( + "os" + "syscall" + "time" + + "github.com/ncw/rclone/fs" +) + +const ( + ERROR_SHARING_VIOLATION syscall.Errno = 32 +) + +// Removes name, retrying on a sharing violation +func remove(name string) (err error) { + const maxTries = 10 + var sleepTime = 1 * time.Millisecond + for i := 0; i < maxTries; i++ { + err = os.Remove(name) + if err == nil { + break + } + pathErr, ok := err.(*os.PathError) + if !ok { + break + } + if pathErr.Err != ERROR_SHARING_VIOLATION { + break + } + fs.Logf(name, "Remove detected sharing violation - retry %d/%d sleeping %v", i+1, maxTries, sleepTime) + time.Sleep(sleepTime) + sleepTime <<= 1 + } + return err +}