os: when looping in RemoveAll, close and re-open directory
On some systems removing files can cause a directory to be re-shuffled,
so simply continuing to read files can cause us to miss some.
Close and re-open the directory when looping, to avoid that.
Read more files each time through the loop, to reduce the chance of
having to re-open.
Fixes #20841
Change-Id: I98a14774ca63786ad05ba5000cbdb01ad2884332
Reviewed-on: https://go-review.googlesource.com/121255
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
diff --git a/src/os/path.go b/src/os/path.go
index 5c53506..cdfbc18 100644
--- a/src/os/path.go
+++ b/src/os/path.go
@@ -6,7 +6,6 @@
import (
"io"
- "runtime"
"syscall"
)
@@ -84,32 +83,35 @@
return err
}
- // Directory.
- fd, err := Open(path)
- if err != nil {
- if IsNotExist(err) {
- // Race. It was deleted between the Lstat and Open.
- // Return nil per RemoveAll's docs.
- return nil
- }
- return err
- }
-
// Remove contents & return first error.
err = nil
for {
- if err == nil && (runtime.GOOS == "plan9" || runtime.GOOS == "nacl") {
- // Reset read offset after removing directory entries.
- // See golang.org/issue/22572.
- fd.Seek(0, 0)
+ fd, err := Open(path)
+ if err != nil {
+ if IsNotExist(err) {
+ // Already deleted by someone else.
+ return nil
+ }
+ return err
}
- names, err1 := fd.Readdirnames(100)
+
+ const request = 1024
+ names, err1 := fd.Readdirnames(request)
+
+ // Removing files from the directory may have caused
+ // the OS to reshuffle it. Simply calling Readdirnames
+ // again may skip some entries. The only reliable way
+ // to avoid this is to close and re-open the
+ // directory. See issue 20841.
+ fd.Close()
+
for _, name := range names {
err1 := RemoveAll(path + string(PathSeparator) + name)
if err == nil {
err = err1
}
}
+
if err1 == io.EOF {
break
}
@@ -120,10 +122,29 @@
if len(names) == 0 {
break
}
- }
- // Close directory, because windows won't remove opened directory.
- fd.Close()
+ // We don't want to re-open unnecessarily, so if we
+ // got fewer than request names from Readdirnames, try
+ // simply removing the directory now. If that
+ // succeeds, we are done.
+ if len(names) < request {
+ err1 := Remove(path)
+ if err1 == nil || IsNotExist(err1) {
+ return nil
+ }
+
+ if err != nil {
+ // We got some error removing the
+ // directory contents, and since we
+ // read fewer names than we requested
+ // there probably aren't more files to
+ // remove. Don't loop around to read
+ // the directory again. We'll probably
+ // just get the same error.
+ return err
+ }
+ }
+ }
// Remove directory.
err1 := Remove(path)
