os: don't try to put directory into non-blocking mode

Fixes #56843

Change-Id: I3cb3e8397499cd8c57a3edddd45f38c510519b36
Reviewed-on: https://go-review.googlesource.com/c/go/+/451997
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Nicolas Hillegeer <aktau@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Rob Pike <r@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/os/file_unix.go b/src/os/file_unix.go
index 1833c26531602b2256e0fa7ad44c1875a5525330..6a884a29a86bed07d055cdd2ec3d0d1fccb158bb 100644 (file)
--- a/src/os/file_unix.go
+++ b/src/os/file_unix.go
@@ -110,10 +110,20 @@ func NewFile(fd uintptr, name string) *File {
 type newFileKind int
 
 const (
+       // kindNewFile means that the descriptor was passed to us via NewFile.
        kindNewFile newFileKind = iota
+       // kindOpenFile means that the descriptor was opened using
+       // Open, Create, or OpenFile.
        kindOpenFile
+       // kindPipe means that the descriptor was opened using Pipe.
        kindPipe
+       // kindNonBlock means that the descriptor was passed to us via NewFile,
+       // and the descriptor is already in non-blocking mode.
        kindNonBlock
+       // kindNoPoll means that we should not put the descriptor into
+       // non-blocking mode, because we know it is not a pipe or FIFO.
+       // Used by openFdAt for directories.
+       kindNoPoll
 )
 
 // newFile is like NewFile, but if called from OpenFile or Pipe

diff --git a/src/os/removeall_at.go b/src/os/removeall_at.go
index 306debd9727907cd7a2914943533a0a5e9fd53a8..378733ffdb4d636011a1bf143f34e676e1d29c55 100644 (file)
--- a/src/os/removeall_at.go
+++ b/src/os/removeall_at.go
@@ -194,5 +194,6 @@ func openFdAt(dirfd int, name string) (*File, error) {
                syscall.CloseOnExec(r)
        }
 
-       return newFile(uintptr(r), name, kindOpenFile), nil
+       // We use kindNoPoll because we know that this is a directory.
+       return newFile(uintptr(r), name, kindNoPoll), nil
 }

diff --git a/src/os/removeall_test.go b/src/os/removeall_test.go
index aa1c04325d367f0d839b1fd651dbf3276b93120a..a3af52cc17cb7e7d84b1cda856bba1ec56cac4fe 100644 (file)
--- a/src/os/removeall_test.go
+++ b/src/os/removeall_test.go
@@ -5,11 +5,14 @@
 package os_test
 
 import (
+       "bytes"
        "fmt"
+       "internal/testenv"
        "os"
        . "os"
        "path/filepath"
        "runtime"
+       "strconv"
        "strings"
        "testing"
 )
@@ -438,3 +441,64 @@ func TestRemoveAllWithMoreErrorThanReqSize(t *testing.T) {
                t.Fatalf("RemoveAll(<read-only directory>) unexpectedly removed %d read-only files from that directory", 1025-len(names))
        }
 }
+
+func TestRemoveAllNoFcntl(t *testing.T) {
+       if testing.Short() {
+               t.Skip("skipping in short mode")
+       }
+
+       const env = "GO_TEST_REMOVE_ALL_NO_FCNTL"
+       if dir := Getenv(env); dir != "" {
+               if err := RemoveAll(dir); err != nil {
+                       t.Fatal(err)
+               }
+               return
+       }
+
+       // Only test on Linux so that we can assume we have strace.
+       // The code is OS-independent so if it passes on Linux
+       // it should pass on other Unix systems.
+       if runtime.GOOS != "linux" {
+               t.Skipf("skipping test on %s", runtime.GOOS)
+       }
+       if _, err := Stat("/bin/strace"); err != nil {
+               t.Skipf("skipping test because /bin/strace not found: %v", err)
+       }
+       me, err := Executable()
+       if err != nil {
+               t.Skipf("skipping because Executable failed: %v", err)
+       }
+
+       // Create 100 directories.
+       // The test is that we can remove them without calling fcntl
+       // on each one.
+       tmpdir := t.TempDir()
+       subdir := filepath.Join(tmpdir, "subdir")
+       if err := Mkdir(subdir, 0o755); err != nil {
+               t.Fatal(err)
+       }
+       for i := 0; i < 100; i++ {
+               subsubdir := filepath.Join(subdir, strconv.Itoa(i))
+               if err := Mkdir(filepath.Join(subdir, strconv.Itoa(i)), 0o755); err != nil {
+                       t.Fatal(err)
+               }
+               if err := WriteFile(filepath.Join(subsubdir, "file"), nil, 0o644); err != nil {
+                       t.Fatal(err)
+               }
+       }
+
+       cmd := testenv.Command(t, "/bin/strace", "-f", "-e", "fcntl", me, "-test.run=TestRemoveAllNoFcntl")
+       cmd = testenv.CleanCmdEnv(cmd)
+       cmd.Env = append(cmd.Env, env+"="+subdir)
+       out, err := cmd.CombinedOutput()
+       if len(out) > 0 {
+               t.Logf("%s", out)
+       }
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       if got := bytes.Count(out, []byte("fcntl")); got >= 100 {
+               t.Errorf("found %d fcntl calls, want < 100", got)
+       }
+}