From: Andy Pan <i@andypan.me>
Date: Fri, 16 Aug 2024 03:42:25 +0000 (+0800)
Subject: os: only employ sendfile(3ext) on illumos when target is regular file
X-Git-Tag: go1.24rc1~1181
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=96ef6abcca2f3ba43ae822d5a6e46fc02092e0a3;p=gostls13.git

os: only employ sendfile(3ext) on illumos when target is regular file

Follows up CL 605355
Fixes #68863

Change-Id: I56e05822502e66eed610d5e924d110607ce146b5
Reviewed-on: https://go-review.googlesource.com/c/go/+/606135
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Andy Pan <panjf2000@gmail.com>
---

diff --git a/src/os/readfrom_unix_test.go b/src/os/readfrom_unix_test.go
index 35e3ab43b8..dbe2b683a7 100644
--- a/src/os/readfrom_unix_test.go
+++ b/src/os/readfrom_unix_test.go
@@ -198,14 +198,16 @@ func TestCopyFile(t *testing.T) {
 				}
 				switch runtime.GOOS {
 				case "illumos", "solaris":
-					// On SunOS, We rely on File.Stat to get the size of the source file,
+					// On solaris, We rely on File.Stat to get the size of the source file,
 					// which doesn't work for pipe.
+					// On illumos, We skip anything other than regular files conservatively
+					// for the target file, therefore the hook shouldn't have been called.
 					if hook.called {
-						t.Fatalf("%s: shouldn't have called the hook with a source of pipe", testName)
+						t.Fatalf("%s: shouldn't have called the hook with a source or a destination of pipe", testName)
 					}
 				default:
 					if !hook.called {
-						t.Fatalf("%s: should have called the hook with a source of pipe", testName)
+						t.Fatalf("%s: should have called the hook with both source and destination of pipe", testName)
 					}
 				}
 				pw2.Close()
@@ -231,8 +233,17 @@ func TestCopyFile(t *testing.T) {
 				if n != int64(len(data)) {
 					t.Fatalf("%s: transferred %d, want %d", testName, n, len(data))
 				}
-				if !hook.called {
-					t.Fatalf("%s: should have called the hook", testName)
+				switch runtime.GOOS {
+				case "illumos":
+					// On illumos, We skip anything other than regular files conservatively
+					// for the target file, therefore the hook shouldn't have been called.
+					if hook.called {
+						t.Fatalf("%s: shouldn't have called the hook with a destination of pipe", testName)
+					}
+				default:
+					if !hook.called {
+						t.Fatalf("%s: should have called the hook with a destination of pipe", testName)
+					}
 				}
 				pw.Close()
 				mustContainData(t, pr, data)
diff --git a/src/os/zero_copy_solaris.go b/src/os/zero_copy_solaris.go
index 9fb659024e..697a368d21 100644
--- a/src/os/zero_copy_solaris.go
+++ b/src/os/zero_copy_solaris.go
@@ -58,7 +58,7 @@ func (f *File) readFrom(r io.Reader) (written int64, handled bool, err error) {
 
 	// sendfile() on illumos seems to incur intermittent failures when the
 	// target file is a standard stream (stdout/stderr), we hereby skip any
-	// character devices conservatively and leave them to generic copy.
+	// anything other than regular files conservatively and leave them to generic copy.
 	// Check out https://go.dev/issue/68863 for more details.
 	if runtime.GOOS == "illumos" {
 		fi, err := f.Stat()
@@ -69,7 +69,7 @@ func (f *File) readFrom(r io.Reader) (written int64, handled bool, err error) {
 		if !ok {
 			return 0, false, nil
 		}
-		if typ := st.Mode & syscall.S_IFMT; typ == syscall.S_IFCHR || typ == syscall.S_IFBLK {
+		if typ := st.Mode & syscall.S_IFMT; typ != syscall.S_IFREG {
 			return 0, false, nil
 		}
 	}