From 2dede2fdc20aebc8a332e79e8e04f32e004eb6df Mon Sep 17 00:00:00 2001
From: Harshavardhana <harsha@minio.io>
Date: Sun, 5 Aug 2018 21:15:28 -0700
Subject: [PATCH] Add reliable RemoveAll to handle racy situations (#6227)

---
 cmd/format-xl.go       |  6 +++---
 cmd/fs-v1-helpers.go   |  2 +-
 cmd/os-reliable.go     | 47 ++++++++++++++++++++++++++++++++++++++++++
 cmd/prepare-storage.go |  4 ++--
 4 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/cmd/format-xl.go b/cmd/format-xl.go
index 41f9008c6..e4d6c60e7 100644
--- a/cmd/format-xl.go
+++ b/cmd/format-xl.go
@@ -21,7 +21,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
-	"os"
 	"reflect"
 	"sync"
 
@@ -267,10 +266,11 @@ func formatXLMigrateV2ToV3(export string) error {
 		return err
 	}
 
-	if err = os.RemoveAll(pathJoin(export, minioMetaMultipartBucket)); err != nil {
+	if err = removeAll(pathJoin(export, minioMetaMultipartBucket)); err != nil {
 		return err
 	}
-	if err = os.MkdirAll(pathJoin(export, minioMetaMultipartBucket), 0755); err != nil {
+
+	if err = mkdirAll(pathJoin(export, minioMetaMultipartBucket), 0755); err != nil {
 		return err
 	}
 
diff --git a/cmd/fs-v1-helpers.go b/cmd/fs-v1-helpers.go
index 3a797127e..d2cfb2f14 100644
--- a/cmd/fs-v1-helpers.go
+++ b/cmd/fs-v1-helpers.go
@@ -64,7 +64,7 @@ func fsRemoveAll(ctx context.Context, dirPath string) (err error) {
 		return err
 	}
 
-	if err = os.RemoveAll(dirPath); err != nil {
+	if err = removeAll(dirPath); err != nil {
 		if os.IsPermission(err) {
 			logger.LogIf(ctx, errVolumeAccessDenied)
 			return errVolumeAccessDenied
diff --git a/cmd/os-reliable.go b/cmd/os-reliable.go
index 627a3065a..cb3b39f24 100644
--- a/cmd/os-reliable.go
+++ b/cmd/os-reliable.go
@@ -22,6 +22,53 @@ import (
 	"path"
 )
 
+// Wrapper functions to os.RemoveAll, which calls reliableRemoveAll
+// this is to ensure that if there is a racy parent directory
+// create in between we can simply retry the operation.
+func removeAll(dirPath string) (err error) {
+	if dirPath == "" {
+		return errInvalidArgument
+	}
+
+	if err = checkPathLength(dirPath); err != nil {
+		return err
+	}
+
+	if err = reliableRemoveAll(dirPath); err != nil {
+		switch {
+		case isSysErrNotDir(err):
+			// File path cannot be verified since one of
+			// the parents is a file.
+			return errFileAccessDenied
+		case isSysErrPathNotFound(err):
+			// This is a special case should be handled only for
+			// windows, because windows API does not return "not a
+			// directory" error message. Handle this specifically
+			// here.
+			return errFileAccessDenied
+		}
+	}
+	return err
+}
+
+// Reliably retries os.RemoveAll if for some reason os.RemoveAll returns
+// syscall.ENOTEMPTY (children has files).
+func reliableRemoveAll(dirPath string) (err error) {
+	i := 0
+	for {
+		// Removes all the directories and files.
+		if err = os.RemoveAll(dirPath); err != nil {
+			// Retry only for the first retryable error.
+			if isSysErrNotEmpty(err) && i == 0 {
+				i++
+				continue
+			}
+		}
+		break
+	}
+	return err
+}
+
 // Wrapper functions to os.MkdirAll, which calls reliableMkdirAll
 // this is to ensure that if there is a racy parent directory
 // delete in between we can simply retry the operation.
diff --git a/cmd/prepare-storage.go b/cmd/prepare-storage.go
index eb6b8fa81..6f310f8d2 100644
--- a/cmd/prepare-storage.go
+++ b/cmd/prepare-storage.go
@@ -80,10 +80,10 @@ func formatXLCleanupTmpLocalEndpoints(endpoints EndpointList) error {
 			}
 			return err
 		}
-		if err := os.RemoveAll(pathJoin(endpoint.Path, minioMetaTmpBucket)); err != nil {
+		if err := removeAll(pathJoin(endpoint.Path, minioMetaTmpBucket)); err != nil {
 			return err
 		}
-		if err := os.MkdirAll(pathJoin(endpoint.Path, minioMetaTmpBucket), 0777); err != nil {
+		if err := mkdirAll(pathJoin(endpoint.Path, minioMetaTmpBucket), 0777); err != nil {
 			return err
 		}
 	}