From 8f2a9267c814538485dd3459910bc8e6de2ef2bb Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Mon, 4 Dec 2017 11:16:51 -0800 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: Ian Lance Taylor --- src/net/pipe.go | 46 ++++++++++++++++++-------------------------- src/net/pipe_test.go | 26 +++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 27 deletions(-) diff --git a/src/net/pipe.go b/src/net/pipe.go index 38d0f42a70..9177fc4036 100644 --- 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 -} +type timeoutError struct{} -func (pe pipeError) Error() string { return pe.errStr } -func (pe pipeError) Timeout() bool { return pe.timeout } -func (pe pipeError) Temporary() bool { return pe.timeout } - -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,11 +202,11 @@ 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 @@ -222,7 +214,7 @@ func (p *pipe) write(b []byte) (n int, err error) { 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,7 +223,7 @@ 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 @@ -239,7 +231,7 @@ func (p *pipe) SetReadDeadline(t time.Time) error { 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 index 382eca7520..84a71b756b 100644 --- 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) + } +}