net/http: refactor ResponseWriter.ReadFrom to permit splice on Linux
Rather than probe and guess if sendfile will work inside ResponseWriter.ReadFrom(src),
this change fixes the underlying issue of starting to respond before src is readable
We'll no longer send a status OK if a header has not yet been written and reading
from src is destined to fail. This small change implicitly takes care of the need for
the server to sniff the response body to determine the Content-Type.
This allows splice to work on Linux when src is a socket or any non-regular file that's spliceable.
The extra read of 512 bytes may raise an objection, and that's fair, but
we're already swapping some syscall prep work for another and a read of
512 probably will not impact the overall performance. For shorter
bodies, there's likely less setup time. A little initial slop is not too
unusual in zero copy network code, and sometimes actually helps.
Fixes #40888
Change-Id: I4a8e2ad0ace1318bae66dae5671d06ea6d4838ed
GitHub-Last-Rev: 097364ea866613d103a31e2247b44f4a12077f9e
GitHub-Pull-Request: golang/go#40903
Reviewed-on: https://go-review.googlesource.com/c/go/+/249238
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
diff --git a/src/net/http/fs_test.go b/src/net/http/fs_test.go
index c082cee..245d9ce 100644
--- a/src/net/http/fs_test.go
+++ b/src/net/http/fs_test.go
@@ -1136,6 +1136,14 @@
t.Skipf("skipping; failed to run strace: %v", err)
}
+ filename := fmt.Sprintf("1kb-%d", os.Getpid())
+ filepath := path.Join(os.TempDir(), filename)
+
+ if err := ioutil.WriteFile(filepath, bytes.Repeat([]byte{'a'}, 1<<10), 0755); err != nil {
+ t.Fatal(err)
+ }
+ defer os.Remove(filepath)
+
var buf bytes.Buffer
child := exec.Command("strace", "-f", "-q", os.Args[0], "-test.run=TestLinuxSendfileChild")
child.ExtraFiles = append(child.ExtraFiles, lnf)
@@ -1146,7 +1154,7 @@
t.Skipf("skipping; failed to start straced child: %v", err)
}
- res, err := Get(fmt.Sprintf("http://%s/", ln.Addr()))
+ res, err := Get(fmt.Sprintf("http://%s/%s", ln.Addr(), filename))
if err != nil {
t.Fatalf("http client error: %v", err)
}
@@ -1192,7 +1200,7 @@
panic(err)
}
mux := NewServeMux()
- mux.Handle("/", FileServer(Dir("testdata")))
+ mux.Handle("/", FileServer(Dir(os.TempDir())))
mux.HandleFunc("/quit", func(ResponseWriter, *Request) {
os.Exit(0)
})
diff --git a/src/net/http/server.go b/src/net/http/server.go
index ed5de35..9124903 100644
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -561,50 +561,52 @@
io.Writer
}
-func srcIsRegularFile(src io.Reader) (isRegular bool, err error) {
- switch v := src.(type) {
- case *os.File:
- fi, err := v.Stat()
- if err != nil {
- return false, err
- }
- return fi.Mode().IsRegular(), nil
- case *io.LimitedReader:
- return srcIsRegularFile(v.R)
- default:
- return
- }
-}
-
// ReadFrom is here to optimize copying from an *os.File regular file
-// to a *net.TCPConn with sendfile.
+// to a *net.TCPConn with sendfile, or from a supported src type such
+// as a *net.TCPConn on Linux with splice.
func (w *response) ReadFrom(src io.Reader) (n int64, err error) {
+ bufp := copyBufPool.Get().(*[]byte)
+ buf := *bufp
+ defer copyBufPool.Put(bufp)
+
// Our underlying w.conn.rwc is usually a *TCPConn (with its
- // own ReadFrom method). If not, or if our src isn't a regular
- // file, just fall back to the normal copy method.
+ // own ReadFrom method). If not, just fall back to the normal
+ // copy method.
rf, ok := w.conn.rwc.(io.ReaderFrom)
- regFile, err := srcIsRegularFile(src)
- if err != nil {
- return 0, err
- }
- if !ok || !regFile {
- bufp := copyBufPool.Get().(*[]byte)
- defer copyBufPool.Put(bufp)
- return io.CopyBuffer(writerOnly{w}, src, *bufp)
+ if !ok {
+ return io.CopyBuffer(writerOnly{w}, src, buf)
}
// sendfile path:
- if !w.wroteHeader {
- w.WriteHeader(StatusOK)
+ // Do not start actually writing response until src is readable.
+ // If body length is <= sniffLen, sendfile/splice path will do
+ // little anyway. This small read also satisfies sniffing the
+ // body in case Content-Type is missing.
+ nr, er := src.Read(buf[:sniffLen])
+ atEOF := errors.Is(er, io.EOF)
+ n += int64(nr)
+
+ if nr > 0 {
+ // Write the small amount read normally.
+ nw, ew := w.Write(buf[:nr])
+ if ew != nil {
+ err = ew
+ } else if nr != nw {
+ err = io.ErrShortWrite
+ }
+ }
+ if err == nil && er != nil && !atEOF {
+ err = er
}
- if w.needsSniff() {
- n0, err := io.Copy(writerOnly{w}, io.LimitReader(src, sniffLen))
- n += n0
- if err != nil {
- return n, err
- }
+ // Do not send StatusOK in the error case where nothing has been written.
+ if err == nil && !w.wroteHeader {
+ w.WriteHeader(StatusOK) // nr == 0, no error (or EOF)
+ }
+
+ if err != nil || atEOF {
+ return n, err
}
w.w.Flush() // get rid of any previous writes
