From 52af6550c92b1023e64c48e7b0fd947e539fb30f Mon Sep 17 00:00:00 2001
From: Ian Lance Taylor <iant@golang.org>
Date: Fri, 18 Nov 2022 15:20:06 -0800
Subject: [PATCH] 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>
---
src/os/file_unix.go | 10 +++++++
src/os/removeall_at.go | 3 +-
src/os/removeall_test.go | 64 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 76 insertions(+), 1 deletion(-)
diff --git a/src/os/file_unix.go b/src/os/file_unix.go
index 1833c265316..6a884a29a86 100644
--- 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 306debd9727..378733ffdb4 100644
--- 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 aa1c04325d3..a3af52cc17c 100644
--- 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)
+ }
+}
--
GitLab