From c043fc4f655ce34f67a0e7fe2833139f6313a3f0 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Wed, 19 Dec 2018 16:47:50 -0800 Subject: [PATCH] os: don't let sendFile put a pipe into blocking mode Use SyscallConn to avoid calling the Fd method in sendFile on Unix systems, since Fd has the side effect of putting the descriptor into blocking mode. Fixes #28330 Change-Id: If093417a225fe44092bd2c0dbbc3937422e98c0b Reviewed-on: https://go-review.googlesource.com/c/155137 Run-TryBot: Ian Lance Taylor TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/net/sendfile_linux.go | 14 ++++- src/net/sendfile_test.go | 104 +++++++++++++++++++++++++++++++++++ src/net/sendfile_unix_alt.go | 14 ++++- 3 files changed, 130 insertions(+), 2 deletions(-) diff --git a/src/net/sendfile_linux.go b/src/net/sendfile_linux.go index c537ea68b2..297e625d24 100644 --- a/src/net/sendfile_linux.go +++ b/src/net/sendfile_linux.go @@ -32,7 +32,19 @@ func sendFile(c *netFD, r io.Reader) (written int64, err error, handled bool) { return 0, nil, false } - written, err = poll.SendFile(&c.pfd, int(f.Fd()), remain) + sc, err := f.SyscallConn() + if err != nil { + return 0, nil, false + } + + var werr error + err = sc.Read(func(fd uintptr) bool { + written, werr = poll.SendFile(&c.pfd, int(fd), remain) + return true + }) + if werr == nil { + werr = err + } if lr != nil { lr.N = remain - written diff --git a/src/net/sendfile_test.go b/src/net/sendfile_test.go index f133744a66..911e6139c5 100644 --- a/src/net/sendfile_test.go +++ b/src/net/sendfile_test.go @@ -12,8 +12,12 @@ import ( "encoding/hex" "fmt" "io" + "io/ioutil" "os" + "runtime" + "sync" "testing" + "time" ) const ( @@ -210,3 +214,103 @@ func TestSendfileSeeked(t *testing.T) { t.Error(err) } } + +// Test that sendfile doesn't put a pipe into blocking mode. +func TestSendfilePipe(t *testing.T) { + switch runtime.GOOS { + case "nacl", "plan9", "windows": + // These systems don't support deadlines on pipes. + t.Skipf("skipping on %s", runtime.GOOS) + } + + t.Parallel() + + ln, err := newLocalListener("tcp") + if err != nil { + t.Fatal(err) + } + defer ln.Close() + + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + defer w.Close() + defer r.Close() + + copied := make(chan bool) + + var wg sync.WaitGroup + wg.Add(1) + go func() { + // Accept a connection and copy 1 byte from the read end of + // the pipe to the connection. This will call into sendfile. + defer wg.Done() + conn, err := ln.Accept() + if err != nil { + t.Error(err) + return + } + defer conn.Close() + _, err = io.CopyN(conn, r, 1) + if err != nil { + t.Error(err) + return + } + // Signal the main goroutine that we've copied the byte. + close(copied) + }() + + wg.Add(1) + go func() { + // Write 1 byte to the write end of the pipe. + defer wg.Done() + _, err := w.Write([]byte{'a'}) + if err != nil { + t.Error(err) + } + }() + + wg.Add(1) + go func() { + // Connect to the server started two goroutines up and + // discard any data that it writes. + defer wg.Done() + conn, err := Dial("tcp", ln.Addr().String()) + if err != nil { + t.Error(err) + return + } + defer conn.Close() + io.Copy(ioutil.Discard, conn) + }() + + // Wait for the byte to be copied, meaning that sendfile has + // been called on the pipe. + <-copied + + // Set a very short deadline on the read end of the pipe. + if err := r.SetDeadline(time.Now().Add(time.Microsecond)); err != nil { + t.Fatal(err) + } + + wg.Add(1) + go func() { + // Wait for much longer than the deadline and write a byte + // to the pipe. + defer wg.Done() + time.Sleep(50 * time.Millisecond) + w.Write([]byte{'b'}) + }() + + // If this read does not time out, the pipe was incorrectly + // put into blocking mode. + _, err = r.Read(make([]byte, 1)) + if err == nil { + t.Error("Read did not time out") + } else if !os.IsTimeout(err) { + t.Errorf("got error %v, expected a time out", err) + } + + wg.Wait() +} diff --git a/src/net/sendfile_unix_alt.go b/src/net/sendfile_unix_alt.go index 9b3ba4ee62..43df3bfd15 100644 --- a/src/net/sendfile_unix_alt.go +++ b/src/net/sendfile_unix_alt.go @@ -58,7 +58,19 @@ func sendFile(c *netFD, r io.Reader) (written int64, err error, handled bool) { return 0, err, false } - written, err = poll.SendFile(&c.pfd, int(f.Fd()), pos, remain) + sc, err := f.SyscallConn() + if err != nil { + return 0, nil, false + } + + var werr error + err = sc.Read(func(fd uintptr) bool { + written, werr = poll.SendFile(&c.pfd, int(fd), pos, remain) + return true + }) + if werr == nil { + werr = err + } if lr != nil { lr.N = remain - written