Commit af37332d authored by Joe Tsai's avatar Joe Tsai Committed by Joe Tsai

io: fix Pipe regression with differing error types

Usage of atomic.Value has a subtle requirement that the
value be of the same concrete type. In prior usage, the intention
was to consistently store a value of the error type.
Since error is an interface, the underlying concrete can differ.

Fix this by creating a type-safe abstraction over atomic.Value
that wraps errors in a struct{error} type to ensure consistent types.

Change-Id: Ica74f2daba15e4cff48d2b4f830d2cb51c608fb6
Reviewed-on: https://go-review.googlesource.com/75594
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: 's avatarIan Lance Taylor <iant@golang.org>
parent 41d860cf
...@@ -13,6 +13,18 @@ import ( ...@@ -13,6 +13,18 @@ import (
"sync/atomic" "sync/atomic"
) )
// atomicError is a type-safe atomic value for errors.
// We use a struct{ error } to ensure consistent use of a concrete type.
type atomicError struct{ v atomic.Value }
func (a *atomicError) Store(err error) {
a.v.Store(struct{ error }{err})
}
func (a *atomicError) Load() error {
err, _ := a.v.Load().(struct{ error })
return err.error
}
// ErrClosedPipe is the error used for read or write operations on a closed pipe. // ErrClosedPipe is the error used for read or write operations on a closed pipe.
var ErrClosedPipe = errors.New("io: read/write on closed pipe") var ErrClosedPipe = errors.New("io: read/write on closed pipe")
...@@ -24,8 +36,8 @@ type pipe struct { ...@@ -24,8 +36,8 @@ type pipe struct {
once sync.Once // Protects closing done once sync.Once // Protects closing done
done chan struct{} done chan struct{}
rerr atomic.Value rerr atomicError
werr atomic.Value werr atomicError
} }
func (p *pipe) Read(b []byte) (n int, err error) { func (p *pipe) Read(b []byte) (n int, err error) {
...@@ -46,8 +58,8 @@ func (p *pipe) Read(b []byte) (n int, err error) { ...@@ -46,8 +58,8 @@ func (p *pipe) Read(b []byte) (n int, err error) {
} }
func (p *pipe) readCloseError() error { func (p *pipe) readCloseError() error {
_, rok := p.rerr.Load().(error) rerr := p.rerr.Load()
if werr, wok := p.werr.Load().(error); !rok && wok { if werr := p.werr.Load(); rerr == nil && werr != nil {
return werr return werr
} }
return ErrClosedPipe return ErrClosedPipe
...@@ -85,8 +97,8 @@ func (p *pipe) Write(b []byte) (n int, err error) { ...@@ -85,8 +97,8 @@ func (p *pipe) Write(b []byte) (n int, err error) {
} }
func (p *pipe) writeCloseError() error { func (p *pipe) writeCloseError() error {
_, wok := p.werr.Load().(error) werr := p.werr.Load()
if rerr, rok := p.rerr.Load().(error); !wok && rok { if rerr := p.rerr.Load(); werr == nil && rerr != nil {
return rerr return rerr
} }
return ErrClosedPipe return ErrClosedPipe
......
...@@ -316,6 +316,31 @@ func TestWriteAfterWriterClose(t *testing.T) { ...@@ -316,6 +316,31 @@ func TestWriteAfterWriterClose(t *testing.T) {
} }
} }
func TestPipeCloseError(t *testing.T) {
type testError1 struct{ error }
type testError2 struct{ error }
r, w := Pipe()
r.CloseWithError(testError1{})
if _, err := w.Write(nil); err != (testError1{}) {
t.Errorf("Write error: got %T, want testError1", err)
}
r.CloseWithError(testError2{})
if _, err := w.Write(nil); err != (testError2{}) {
t.Errorf("Write error: got %T, want testError2", err)
}
r, w = Pipe()
w.CloseWithError(testError1{})
if _, err := r.Read(nil); err != (testError1{}) {
t.Errorf("Read error: got %T, want testError1", err)
}
w.CloseWithError(testError2{})
if _, err := r.Read(nil); err != (testError2{}) {
t.Errorf("Read error: got %T, want testError2", err)
}
}
func TestPipeConcurrent(t *testing.T) { func TestPipeConcurrent(t *testing.T) {
const ( const (
input = "0123456789abcdef" input = "0123456789abcdef"
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment