[tools] internal/lsp/fake: retry spurious file lock errors on windows

[Robert Findley (Gerrit)]

Robert Findley submitted this change.

View Change

1 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: internal/lsp/fake/workdir_windows.go
Insertions: 8, Deletions: 3.

@@ -6,13 +6,18 @@
 
 import (
 	"errors"
-	"internal/syscall/windows"
 	"syscall"
 )
 
 func init() {
+	// constants copied from GOROOT/src/internal/syscall/windows/syscall_windows.go
+	const (
+		ERROR_SHARING_VIOLATION syscall.Errno = 32
+		ERROR_LOCK_VIOLATION    syscall.Errno = 33
+	)
+
 	isWindowsErrLockViolation = func(err error) bool {
-		return errors.Is(err, windows.ERROR_LOCK_VIOLATION)
+		return errors.Is(err, ERROR_LOCK_VIOLATION)
 	}
 
 	// Copied from GOROOT/src/testing/testing_windows.go
@@ -27,7 +32,7 @@
 		if err == syscall.ERROR_ACCESS_DENIED {
 			return true // Observed in https://go.dev/issue/50051.
 		}
-		if err == windows.ERROR_SHARING_VIOLATION {
+		if err == ERROR_SHARING_VIOLATION {
 			return true // Observed in https://go.dev/issue/51442.
 		}
 		return false
```

Approvals: kokoro: gopls CI succeeded Gopher Robot: TryBots succeeded Robert Findley: Run TryBots Bryan Mills: Looks good to me, approved

internal/lsp/fake: retry spurious file lock errors on windows

Cleaning the regtest sandbox sometimes fails on windows with errors that
may be spurious. Copy logic from the testing package to retry these
errors.

For golang/go#53819

Change-Id: I059fbb5e023af1cd52a5d231cd11a7c2ae72bc92
Reviewed-on: https://go-review.googlesource.com/c/tools/+/417117
Run-TryBot: Robert Findley <rfin...@google.com>
Reviewed-by: Bryan Mills <bcm...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
gopls-CI: kokoro <noreply...@google.com>
---
M internal/lsp/fake/sandbox.go
M internal/lsp/fake/workdir.go
M internal/lsp/fake/workdir_windows.go
3 files changed, 77 insertions(+), 3 deletions(-)

diff --git a/internal/lsp/fake/sandbox.go b/internal/lsp/fake/sandbox.go
index b439564..72b0121 100644
--- a/internal/lsp/fake/sandbox.go
+++ b/internal/lsp/fake/sandbox.go
@@ -9,9 +9,11 @@
 	"errors"
 	"fmt"
 	"io/ioutil"
+	"math/rand"
 	"os"
 	"path/filepath"
 	"strings"
+	"time"
 
 	"golang.org/x/tools/internal/gocommand"
 	"golang.org/x/tools/internal/testenv"
@@ -266,9 +268,36 @@
 	if sb.gopath != "" {
 		goCleanErr = sb.RunGoCommand(context.Background(), "", "clean", []string{"-modcache"}, false)
 	}
-	err := os.RemoveAll(sb.rootdir)
+	err := removeAll(sb.rootdir)
 	if err != nil || goCleanErr != nil {
 		return fmt.Errorf("error(s) cleaning sandbox: cleaning modcache: %v; removing files: %v", goCleanErr, err)
 	}
 	return nil
 }
+
+// removeAll is copied from GOROOT/src/testing/testing.go
+//
+// removeAll is like os.RemoveAll, but retries Windows "Access is denied."
+// errors up to an arbitrary timeout.
+//
+// See https://go.dev/issue/50051 for additional context.
+func removeAll(path string) error {
+	const arbitraryTimeout = 2 * time.Second
+	var (
+		start     time.Time
+		nextSleep = 1 * time.Millisecond
+	)
+	for {
+		err := os.RemoveAll(path)
+		if !isWindowsRetryable(err) {
+			return err
+		}
+		if start.IsZero() {
+			start = time.Now()
+		} else if d := time.Since(start) + nextSleep; d >= arbitraryTimeout {
+			return err
+		}
+		time.Sleep(nextSleep)
+		nextSleep += time.Duration(rand.Int63n(int64(nextSleep)))
+	}
+}
diff --git a/internal/lsp/fake/workdir.go b/internal/lsp/fake/workdir.go
index 734f5fd..0a72083 100644
--- a/internal/lsp/fake/workdir.go
+++ b/internal/lsp/fake/workdir.go
@@ -77,6 +77,10 @@
 // on Windows.
 var isWindowsErrLockViolation = func(err error) bool { return false }
 
+// isWindowsRetryable reports whether err is a Windows error code
+// that may be fixed by retrying a failed filesystem operation.
+var isWindowsRetryable = func(err error) bool { return false }
+
 // Workdir is a temporary working directory for tests. It exposes file
 // operations in terms of relative paths, and fakes file watching by triggering
 // events on file operations.
diff --git a/internal/lsp/fake/workdir_windows.go b/internal/lsp/fake/workdir_windows.go
index bcd18b7..fc5ad1a 100644
--- a/internal/lsp/fake/workdir_windows.go
+++ b/internal/lsp/fake/workdir_windows.go
@@ -10,10 +10,31 @@
 )
 
 func init() {
-	// from https://docs.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499-
-	const ERROR_LOCK_VIOLATION syscall.Errno = 33
+	// constants copied from GOROOT/src/internal/syscall/windows/syscall_windows.go
+	const (
+		ERROR_SHARING_VIOLATION syscall.Errno = 32
+		ERROR_LOCK_VIOLATION    syscall.Errno = 33
+	)
 
 	isWindowsErrLockViolation = func(err error) bool {
 		return errors.Is(err, ERROR_LOCK_VIOLATION)
 	}
+
+	// Copied from GOROOT/src/testing/testing_windows.go
+	isWindowsRetryable = func(err error) bool {
+		for {
+			unwrapped := errors.Unwrap(err)
+			if unwrapped == nil {
+				break
+			}
+			err = unwrapped
+		}
+		if err == syscall.ERROR_ACCESS_DENIED {
+			return true // Observed in https://go.dev/issue/50051.
+		}
+		if err == ERROR_SHARING_VIOLATION {
+			return true // Observed in https://go.dev/issue/51442.
+		}
+		return false
+	}
 }

To view, visit change 417117. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: tools

Gerrit-Branch: master

Gerrit-Change-Id: I059fbb5e023af1cd52a5d231cd11a7c2ae72bc92

Gerrit-Change-Number: 417117

Gerrit-PatchSet: 4

Gerrit-Owner: Robert Findley <rfin...@google.com>

Gerrit-Reviewer: Bryan Mills <bcm...@google.com>

Gerrit-Reviewer: Gopher Robot <go...@golang.org>

Gerrit-Reviewer: Robert Findley <rfin...@google.com>

Gerrit-Reviewer: kokoro <noreply...@google.com>

Gerrit-MessageType: merged