net: return io.ErrClosedPipe when possible from net.Pipe

The previous implementation of net.Pipe was just a thin wrapper around io.Pipe and did not wrap any of the io.Pipe errors as net.Errors. As a result of Hyrum's law, users have come to depend on the fact that net.Pipe returns io.ErrClosedPipe when the pipe is closed. Thus, we preserve this behavior to avoid regressing such use cases. Change-Id: I06b387877b944c1c08527601f58983872b7557b4 Reviewed-on: https://go-review.googlesource.com/81777 Run-TryBot: Joe Tsai <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
achew22 · Dec 5, 2017 · 8f2a926 · 8f2a926
1 parent 49fec9b
commit 8f2a926
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 27 deletions.
diff --git a/src/net/pipe.go b/src/net/pipe.go
@@ -78,19 +78,11 @@ func isClosedChan(c <-chan struct{}) bool {
 	}
 }
 
-type pipeError struct {
-	errStr  string
-	timeout bool
-}
-
-func (pe pipeError) Error() string   { return pe.errStr }
-func (pe pipeError) Timeout() bool   { return pe.timeout }
-func (pe pipeError) Temporary() bool { return pe.timeout }
+type timeoutError struct{}
 
-var (
-	errDeadline = pipeError{"deadline exceeded", true}
-	errClosed   = pipeError{"closed connection", false}
-)
+func (timeoutError) Error() string   { return "deadline exceeded" }
+func (timeoutError) Timeout() bool   { return true }
+func (timeoutError) Temporary() bool { return true }
 
 type pipeAddr struct{}
 
@@ -153,7 +145,7 @@ func (*pipe) RemoteAddr() Addr { return pipeAddr{} }
 
 func (p *pipe) Read(b []byte) (int, error) {
 	n, err := p.read(b)
-	if err != nil && err != io.EOF {
+	if err != nil && err != io.EOF && err != io.ErrClosedPipe {
 		err = &OpError{Op: "read", Net: "pipe", Err: err}
 	}
 	return n, err
@@ -162,11 +154,11 @@ func (p *pipe) Read(b []byte) (int, error) {
 func (p *pipe) read(b []byte) (n int, err error) {
 	switch {
 	case isClosedChan(p.localDone):
-		return 0, errClosed
+		return 0, io.ErrClosedPipe
 	case isClosedChan(p.remoteDone):
 		return 0, io.EOF
 	case isClosedChan(p.readDeadline.wait()):
-		return 0, errDeadline
+		return 0, timeoutError{}
 	}
 
 	select {
@@ -175,17 +167,17 @@ func (p *pipe) read(b []byte) (n int, err error) {
 		p.rdTx <- nr
 		return nr, nil
 	case <-p.localDone:
-		return 0, errClosed
+		return 0, io.ErrClosedPipe
 	case <-p.remoteDone:
 		return 0, io.EOF
 	case <-p.readDeadline.wait():
-		return 0, errDeadline
+		return 0, timeoutError{}
 	}
 }
 
 func (p *pipe) Write(b []byte) (int, error) {
 	n, err := p.write(b)
-	if err != nil {
+	if err != nil && err != io.ErrClosedPipe {
 		err = &OpError{Op: "write", Net: "pipe", Err: err}
 	}
 	return n, err
@@ -194,11 +186,11 @@ func (p *pipe) Write(b []byte) (int, error) {
 func (p *pipe) write(b []byte) (n int, err error) {
 	switch {
 	case isClosedChan(p.localDone):
-		return 0, errClosed
+		return 0, io.ErrClosedPipe
 	case isClosedChan(p.remoteDone):
-		return 0, errClosed
+		return 0, io.ErrClosedPipe
 	case isClosedChan(p.writeDeadline.wait()):
-		return 0, errDeadline
+		return 0, timeoutError{}
 	}
 
 	p.wrMu.Lock() // Ensure entirety of b is written together
@@ -210,19 +202,19 @@ func (p *pipe) write(b []byte) (n int, err error) {
 			b = b[nw:]
 			n += nw
 		case <-p.localDone:
-			return n, errClosed
+			return n, io.ErrClosedPipe
 		case <-p.remoteDone:
-			return n, errClosed
+			return n, io.ErrClosedPipe
 		case <-p.writeDeadline.wait():
-			return n, errDeadline
+			return n, timeoutError{}
 		}
 	}
 	return n, nil
 }
 
 func (p *pipe) SetDeadline(t time.Time) error {
 	if isClosedChan(p.localDone) || isClosedChan(p.remoteDone) {
-		return &OpError{Op: "set", Net: "pipe", Err: errClosed}
+		return io.ErrClosedPipe
 	}
 	p.readDeadline.set(t)
 	p.writeDeadline.set(t)
@@ -231,15 +223,15 @@ func (p *pipe) SetDeadline(t time.Time) error {
 
 func (p *pipe) SetReadDeadline(t time.Time) error {
 	if isClosedChan(p.localDone) || isClosedChan(p.remoteDone) {
-		return &OpError{Op: "set", Net: "pipe", Err: errClosed}
+		return io.ErrClosedPipe
 	}
 	p.readDeadline.set(t)
 	return nil
 }
 
 func (p *pipe) SetWriteDeadline(t time.Time) error {
 	if isClosedChan(p.localDone) || isClosedChan(p.remoteDone) {
-		return &OpError{Op: "set", Net: "pipe", Err: errClosed}
+		return io.ErrClosedPipe
 	}
 	p.writeDeadline.set(t)
 	return nil

diff --git a/src/net/pipe_test.go b/src/net/pipe_test.go
@@ -5,8 +5,10 @@
 package net_test
 
 import (
+	"io"
 	"net"
 	"testing"
+	"time"
 
 	"golang_org/x/net/nettest"
 )
@@ -21,3 +23,27 @@ func TestPipe(t *testing.T) {
 		return
 	})
 }
+
+func TestPipeCloseError(t *testing.T) {
+	c1, c2 := net.Pipe()
+	c1.Close()
+
+	if _, err := c1.Read(nil); err != io.ErrClosedPipe {
+		t.Errorf("c1.Read() = %v, want io.ErrClosedPipe", err)
+	}
+	if _, err := c1.Write(nil); err != io.ErrClosedPipe {
+		t.Errorf("c1.Write() = %v, want io.ErrClosedPipe", err)
+	}
+	if err := c1.SetDeadline(time.Time{}); err != io.ErrClosedPipe {
+		t.Errorf("c1.SetDeadline() = %v, want io.ErrClosedPipe", err)
+	}
+	if _, err := c2.Read(nil); err != io.EOF {
+		t.Errorf("c2.Read() = %v, want io.EOF", err)
+	}
+	if _, err := c2.Write(nil); err != io.ErrClosedPipe {
+		t.Errorf("c2.Write() = %v, want io.ErrClosedPipe", err)
+	}
+	if err := c2.SetDeadline(time.Time{}); err != io.ErrClosedPipe {
+		t.Errorf("c2.SetDeadline() = %v, want io.ErrClosedPipe", err)
+	}
+}