From e2928124d1c0c21b2b0b929de310d1da7225469c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 22 Sep 2020 10:11:46 +0200 Subject: [PATCH] pkg/server: make ensureRemoveAll() an alias for os.RemoveAll() on Windows The tricks performed by ensureRemoveAll only make sense for Linux and other Unices, so separate it out, and make ensureRemoveAll for Windows just an alias of os.RemoveAll. Signed-off-by: Sebastiaan van Stijn --- pkg/server/helpers_windows.go | 59 ++--------------------------------- 1 file changed, 2 insertions(+), 57 deletions(-) diff --git a/pkg/server/helpers_windows.go b/pkg/server/helpers_windows.go index 5ce7104da..f88f34bad 100644 --- a/pkg/server/helpers_windows.go +++ b/pkg/server/helpers_windows.go @@ -23,7 +23,6 @@ import ( "os" "path/filepath" "syscall" - "time" "github.com/opencontainers/runtime-spec/specs-go" ) @@ -161,63 +160,9 @@ func fixLongPath(path string) string { return string(pathbuf[:w]) } -// ensureRemoveAll wraps `os.RemoveAll` to check for specific errors that can -// often be remedied. -// Only use `ensureRemoveAll` if you really want to make every effort to remove -// a directory. -// -// Because of the way `os.Remove` (and by extension `os.RemoveAll`) works, there -// can be a race between reading directory entries and then actually attempting -// to remove everything in the directory. -// These types of errors do not need to be returned since it's ok for the dir to -// be gone we can just retry the remove operation. -// -// This should not return a `os.ErrNotExist` kind of error under any circumstances +// ensureRemoveAll is a wrapper for os.RemoveAll on Windows. func ensureRemoveAll(_ context.Context, dir string) error { - notExistErr := make(map[string]bool) - - // track retries - exitOnErr := make(map[string]int) - maxRetry := 50 - - for { - err := os.RemoveAll(dir) - if err == nil { - return nil - } - - pe, ok := err.(*os.PathError) - if !ok { - return err - } - - if os.IsNotExist(err) { - if notExistErr[pe.Path] { - return err - } - notExistErr[pe.Path] = true - - // There is a race where some subdir can be removed but after the - // parent dir entries have been read. - // So the path could be from `os.Remove(subdir)` - // If the reported non-existent path is not the passed in `dir` we - // should just retry, but otherwise return with no error. - if pe.Path == dir { - return nil - } - continue - } - - if pe.Err != syscall.EBUSY { - return err - } - - if exitOnErr[pe.Path] == maxRetry { - return err - } - exitOnErr[pe.Path]++ - time.Sleep(100 * time.Millisecond) - } + return os.RemoveAll(dir) } func modifyProcessLabel(runtimeType string, spec *specs.Spec) error {