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 <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/net/sendfile_linux.go b/src/net/sendfile_linux.go
index c537ea68b2b410cadf094a747d27ad719654b7e4..297e625d24b1d86623316abb51c32e5fc313f6d3 100644 (file)
--- 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 f133744a6654547dfc8b4994a5fb048fc581da91..911e6139c57e3f2529f61cd500299556b53749f5 100644 (file)
--- 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 9b3ba4ee624a3cf8d5ce1749466ce029ccf789a9..43df3bfd15eff08ba1a39b2f62febc7917191462 100644 (file)
--- 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