diff --git a/internal/lsp/fake/sandbox.go b/internal/lsp/fake/sandbox.go index b4395646bc..72b01217ae 100644 --- a/internal/lsp/fake/sandbox.go +++ b/internal/lsp/fake/sandbox.go @@ -9,9 +9,11 @@ import ( "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 @@ func (sb *Sandbox) Close() error { 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 734f5fd819..0a72083bf2 100644 --- a/internal/lsp/fake/workdir.go +++ b/internal/lsp/fake/workdir.go @@ -77,6 +77,10 @@ func WriteFileData(path string, content []byte, rel RelativeTo) error { // 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 bcd18b7a22..fc5ad1a89a 100644 --- a/internal/lsp/fake/workdir_windows.go +++ b/internal/lsp/fake/workdir_windows.go @@ -10,10 +10,31 @@ import ( ) 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 + } }