]> Cypherpunks repositories - gostls13.git/commitdiff
net, os, internal/poll: test for use of sendfile
authorDamien Neil <dneil@google.com>
Thu, 25 Apr 2024 18:53:45 +0000 (11:53 -0700)
committerDamien Neil <dneil@google.com>
Fri, 26 Apr 2024 18:12:56 +0000 (18:12 +0000)
The net package's sendfile tests exercise various paths where
we expect sendfile to be used, but don't verify that sendfile
was in fact used.

Add a hook to internal/poll.SendFile to let us verify that
sendfile was called when expected. Update os package tests
(which use their own hook mechanism) to use this hook as well.

For #66988

Change-Id: I7afb130dcfe0063d60c6ea0f8560cf8665ad5a81
Reviewed-on: https://go-review.googlesource.com/c/go/+/581778
Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

13 files changed:
src/internal/poll/sendfile.go [new file with mode: 0644]
src/internal/poll/sendfile_bsd.go
src/internal/poll/sendfile_linux.go
src/internal/poll/sendfile_solaris.go
src/internal/poll/sendfile_windows.go
src/net/sendfile_linux.go
src/net/sendfile_stub.go
src/net/sendfile_test.go
src/net/sendfile_unix_alt.go
src/net/sendfile_windows.go
src/os/export_linux_test.go
src/os/writeto_linux_test.go
src/os/zero_copy_linux.go

diff --git a/src/internal/poll/sendfile.go b/src/internal/poll/sendfile.go
new file mode 100644 (file)
index 0000000..41b0481
--- /dev/null
@@ -0,0 +1,7 @@
+// Copyright 2024 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package poll
+
+var TestHookDidSendFile = func(dstFD *FD, src int, written int64, err error, handled bool) {}
index 8fcdb1c22e602336af5fa301bad8e5962d7f8360..669df94cc12e0d50d25b9b99a896252377018d38 100644 (file)
@@ -14,6 +14,9 @@ const maxSendfileSize int = 4 << 20
 
 // SendFile wraps the sendfile system call.
 func SendFile(dstFD *FD, src int, pos, remain int64) (written int64, err error, handled bool) {
+       defer func() {
+               TestHookDidSendFile(dstFD, src, written, err, handled)
+       }()
        if err := dstFD.writeLock(); err != nil {
                return 0, err, false
        }
index c2a065329431ecbb6ab3f3b2f7619bf67bb078c4..d1c4d5c0d3d34de9c3cd8d3cb8ae7cccce7bee8d 100644 (file)
@@ -12,6 +12,9 @@ const maxSendfileSize int = 4 << 20
 
 // SendFile wraps the sendfile system call.
 func SendFile(dstFD *FD, src int, remain int64) (written int64, err error, handled bool) {
+       defer func() {
+               TestHookDidSendFile(dstFD, src, written, err, handled)
+       }()
        if err := dstFD.writeLock(); err != nil {
                return 0, err, false
        }
index 1ba0c8d0643bce8325e3397f4ca0b31e3f7783e4..ec675833a225dc2ae5af469c3aa986a307a9a3ab 100644 (file)
@@ -17,6 +17,9 @@ const maxSendfileSize int = 4 << 20
 
 // SendFile wraps the sendfile system call.
 func SendFile(dstFD *FD, src int, pos, remain int64) (written int64, err error, handled bool) {
+       defer func() {
+               TestHookDidSendFile(dstFD, src, written, err, handled)
+       }()
        if err := dstFD.writeLock(); err != nil {
                return 0, err, false
        }
index 8c3353bc6ffa793d93a721a573c7a8672884857e..2ae8a8d1d791e7c90b7eef89826f1bf8f2250259 100644 (file)
@@ -11,6 +11,9 @@ import (
 
 // SendFile wraps the TransmitFile call.
 func SendFile(fd *FD, src syscall.Handle, n int64) (written int64, err error) {
+       defer func() {
+               TestHookDidSendFile(fd, 0, written, err, written > 0)
+       }()
        if fd.kind == kindPipe {
                // TransmitFile does not work with pipes
                return 0, syscall.ESPIPE
index 9a7d0058032f13dd87afcff2806075af4cc1bcf4..f8a7bec8d38169715a622fb11cd72b4a06c75e29 100644 (file)
@@ -10,6 +10,8 @@ import (
        "os"
 )
 
+const supportsSendfile = true
+
 // sendFile copies the contents of r to c using the sendfile
 // system call to minimize copies.
 //
index a4fdd99ffec22c9aaa1063184b8578e955c614c3..7f31cc63e1ed6a909a98270f4611a257e662cffd 100644 (file)
@@ -2,12 +2,14 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-//go:build aix || js || netbsd || openbsd || ios || wasip1
+//go:build !(linux || (darwin && !ios) || dragonfly || freebsd || solaris || windows)
 
 package net
 
 import "io"
 
+const supportsSendfile = false
+
 func sendFile(c *netFD, r io.Reader) (n int64, err error, handled bool) {
        return 0, nil, false
 }
index 8fadb47c153374e3060d96d3e945fd183c5aab17..4f3411565bb6f38aef320da2f0cd65eafda8d0e0 100644 (file)
@@ -11,6 +11,7 @@ import (
        "encoding/hex"
        "errors"
        "fmt"
+       "internal/poll"
        "io"
        "os"
        "runtime"
@@ -26,6 +27,48 @@ const (
        newtonSHA256 = "d4a9ac22462b35e7821a4f2706c211093da678620a8f9997989ee7cf8d507bbd"
 )
 
+// expectSendfile runs f, and verifies that internal/poll.SendFile successfully handles
+// a write to wantConn during f's execution.
+//
+// On platforms where supportsSendfile is false, expectSendfile runs f but does not
+// expect a call to SendFile.
+func expectSendfile(t *testing.T, wantConn Conn, f func()) {
+       t.Helper()
+       if !supportsSendfile {
+               f()
+               return
+       }
+       orig := poll.TestHookDidSendFile
+       defer func() {
+               poll.TestHookDidSendFile = orig
+       }()
+       var (
+               called     bool
+               gotHandled bool
+               gotFD      *poll.FD
+       )
+       poll.TestHookDidSendFile = func(dstFD *poll.FD, src int, written int64, err error, handled bool) {
+               if called {
+                       t.Error("internal/poll.SendFile called multiple times, want one call")
+               }
+               called = true
+               gotHandled = handled
+               gotFD = dstFD
+       }
+       f()
+       if !called {
+               t.Error("internal/poll.SendFile was not called, want it to be")
+               return
+       }
+       if !gotHandled {
+               t.Error("internal/poll.SendFile did not handle the write, want it to")
+               return
+       }
+       if &wantConn.(*TCPConn).fd.pfd != gotFD {
+               t.Error("internal.poll.SendFile called with unexpected FD")
+       }
+}
+
 func TestSendfile(t *testing.T) {
        ln := newLocalListener(t, "tcp")
        defer ln.Close()
@@ -53,7 +96,17 @@ func TestSendfile(t *testing.T) {
 
                        // Return file data using io.Copy, which should use
                        // sendFile if available.
-                       sbytes, err := io.Copy(conn, f)
+                       var sbytes int64
+                       switch runtime.GOOS {
+                       case "windows":
+                               // Windows is not using sendfile for some reason:
+                               // https://go.dev/issue/67042
+                               sbytes, err = io.Copy(conn, f)
+                       default:
+                               expectSendfile(t, conn, func() {
+                                       sbytes, err = io.Copy(conn, f)
+                               })
+                       }
                        if err != nil {
                                errc <- err
                                return
@@ -121,7 +174,9 @@ func TestSendfileParts(t *testing.T) {
                        for i := 0; i < 3; i++ {
                                // Return file data using io.CopyN, which should use
                                // sendFile if available.
-                               _, err = io.CopyN(conn, f, 3)
+                               expectSendfile(t, conn, func() {
+                                       _, err = io.CopyN(conn, f, 3)
+                               })
                                if err != nil {
                                        errc <- err
                                        return
@@ -180,7 +235,9 @@ func TestSendfileSeeked(t *testing.T) {
                                return
                        }
 
-                       _, err = io.CopyN(conn, f, sendSize)
+                       expectSendfile(t, conn, func() {
+                               _, err = io.CopyN(conn, f, sendSize)
+                       })
                        if err != nil {
                                errc <- err
                                return
@@ -240,6 +297,10 @@ func TestSendfilePipe(t *testing.T) {
                        return
                }
                defer conn.Close()
+               // The comment above states that this should call into sendfile,
+               // but empirically it doesn't seem to do so at this time.
+               // If it does, or does on some platforms, this CopyN should be wrapped
+               // in expectSendfile.
                _, err = io.CopyN(conn, r, 1)
                if err != nil {
                        t.Error(err)
@@ -333,6 +394,10 @@ func TestSendfileOnWriteTimeoutExceeded(t *testing.T) {
                }
                defer f.Close()
 
+               // We expect this to use sendfile, but as of the time this comment was written
+               // poll.SendFile on an FD past its timeout can return an error indicating that
+               // it didn't handle the operation, resulting in a non-sendfile retry.
+               // So don't use expectSendfile here.
                _, err = io.Copy(conn, f)
                if errors.Is(err, os.ErrDeadlineExceeded) {
                        return nil
index 5a10540f8af265007a605fb9a607f04563290894..9e46c4e607d4d86999ad9ec9808bfbbf18e8a4b8 100644 (file)
@@ -13,6 +13,8 @@ import (
        "syscall"
 )
 
+const supportsSendfile = true
+
 // sendFile copies the contents of r to c using the sendfile
 // system call to minimize copies.
 //
@@ -35,6 +37,8 @@ func sendFile(c *netFD, r io.Reader) (written int64, err error, handled bool) {
                        return 0, nil, true
                }
        }
+       // r might be an *os.File or an os.fileWithoutWriteTo.
+       // Type assert to an interface rather than *os.File directly to handle the latter case.
        f, ok := r.(interface {
                fs.File
                io.Seeker
index 59b1b0d5c1dd85b0cd540f463184ed7295d3ddc8..0377a485daa02516ab1092a5f7b4e38fc3ff56c9 100644 (file)
@@ -11,6 +11,8 @@ import (
        "syscall"
 )
 
+const supportsSendfile = true
+
 // sendFile copies the contents of r to c using the TransmitFile
 // system call to minimize copies.
 //
index 942b48a17d802d371fc34e7e6dd0f97ce8988092..78c2d144be89bc5c4137cc1f8661a290847361f6 100644 (file)
@@ -7,6 +7,5 @@ package os
 var (
        PollCopyFileRangeP  = &pollCopyFileRange
        PollSpliceFile      = &pollSplice
-       PollSendFile        = &pollSendFile
        GetPollFDAndNetwork = getPollFDAndNetwork
 )
index 5ffab88a2ab52630ba24c9855d4cda8c1515df2c..e3900631baa1ee35bf3484867b87e7785c00361f 100644 (file)
@@ -109,8 +109,18 @@ func newSendFileTest(t *testing.T, proto string, size int64) (net.Conn, *File, n
 
 func hookSendFile(t *testing.T) *sendFileHook {
        h := new(sendFileHook)
-       h.install()
-       t.Cleanup(h.uninstall)
+       orig := poll.TestHookDidSendFile
+       t.Cleanup(func() {
+               poll.TestHookDidSendFile = orig
+       })
+       poll.TestHookDidSendFile = func(dstFD *poll.FD, src int, written int64, err error, handled bool) {
+               h.called = true
+               h.dstfd = dstFD.Sysfd
+               h.srcfd = src
+               h.written = written
+               h.err = err
+               h.handled = handled
+       }
        return h
 }
 
@@ -118,29 +128,10 @@ type sendFileHook struct {
        called bool
        dstfd  int
        srcfd  int
-       remain int64
 
        written int64
        handled bool
        err     error
-
-       original func(dst *poll.FD, src int, remain int64) (int64, error, bool)
-}
-
-func (h *sendFileHook) install() {
-       h.original = *PollSendFile
-       *PollSendFile = func(dst *poll.FD, src int, remain int64) (int64, error, bool) {
-               h.called = true
-               h.dstfd = dst.Sysfd
-               h.srcfd = src
-               h.remain = remain
-               h.written, h.err, h.handled = h.original(dst, src, remain)
-               return h.written, h.err, h.handled
-       }
-}
-
-func (h *sendFileHook) uninstall() {
-       *PollSendFile = h.original
 }
 
 func createTempFile(t *testing.T, size int64) (*File, []byte) {
index 70a05ffa1e15dc29fc05ec8e4124a44fb09eb3a5..0afc19e125a24ef850d7f2e9a943ce7b14032afa 100644 (file)
@@ -13,7 +13,6 @@ import (
 var (
        pollCopyFileRange = poll.CopyFileRange
        pollSplice        = poll.Splice
-       pollSendFile      = poll.SendFile
 )
 
 // wrapSyscallError takes an error and a syscall name. If the error is
@@ -38,7 +37,7 @@ func (f *File) writeTo(w io.Writer) (written int64, handled bool, err error) {
        }
 
        rerr := sc.Read(func(fd uintptr) (done bool) {
-               written, err, handled = pollSendFile(pfd, int(fd), 1<<63-1)
+               written, err, handled = poll.SendFile(pfd, int(fd), 1<<63-1)
                return true
        })