Skip to content

Commit af37332

Browse files
committed
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 <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
1 parent 41d860c commit af37332

File tree

2 files changed

+43
-6
lines changed

2 files changed

+43
-6
lines changed

src/io/pipe.go

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,18 @@ import (
1313
"sync/atomic"
1414
)
1515

16+
// atomicError is a type-safe atomic value for errors.
17+
// We use a struct{ error } to ensure consistent use of a concrete type.
18+
type atomicError struct{ v atomic.Value }
19+
20+
func (a *atomicError) Store(err error) {
21+
a.v.Store(struct{ error }{err})
22+
}
23+
func (a *atomicError) Load() error {
24+
err, _ := a.v.Load().(struct{ error })
25+
return err.error
26+
}
27+
1628
// ErrClosedPipe is the error used for read or write operations on a closed pipe.
1729
var ErrClosedPipe = errors.New("io: read/write on closed pipe")
1830

@@ -24,8 +36,8 @@ type pipe struct {
2436

2537
once sync.Once // Protects closing done
2638
done chan struct{}
27-
rerr atomic.Value
28-
werr atomic.Value
39+
rerr atomicError
40+
werr atomicError
2941
}
3042

3143
func (p *pipe) Read(b []byte) (n int, err error) {
@@ -46,8 +58,8 @@ func (p *pipe) Read(b []byte) (n int, err error) {
4658
}
4759

4860
func (p *pipe) readCloseError() error {
49-
_, rok := p.rerr.Load().(error)
50-
if werr, wok := p.werr.Load().(error); !rok && wok {
61+
rerr := p.rerr.Load()
62+
if werr := p.werr.Load(); rerr == nil && werr != nil {
5163
return werr
5264
}
5365
return ErrClosedPipe
@@ -85,8 +97,8 @@ func (p *pipe) Write(b []byte) (n int, err error) {
8597
}
8698

8799
func (p *pipe) writeCloseError() error {
88-
_, wok := p.werr.Load().(error)
89-
if rerr, rok := p.rerr.Load().(error); !wok && rok {
100+
werr := p.werr.Load()
101+
if rerr := p.rerr.Load(); werr == nil && rerr != nil {
90102
return rerr
91103
}
92104
return ErrClosedPipe

src/io/pipe_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,31 @@ func TestWriteAfterWriterClose(t *testing.T) {
316316
}
317317
}
318318

319+
func TestPipeCloseError(t *testing.T) {
320+
type testError1 struct{ error }
321+
type testError2 struct{ error }
322+
323+
r, w := Pipe()
324+
r.CloseWithError(testError1{})
325+
if _, err := w.Write(nil); err != (testError1{}) {
326+
t.Errorf("Write error: got %T, want testError1", err)
327+
}
328+
r.CloseWithError(testError2{})
329+
if _, err := w.Write(nil); err != (testError2{}) {
330+
t.Errorf("Write error: got %T, want testError2", err)
331+
}
332+
333+
r, w = Pipe()
334+
w.CloseWithError(testError1{})
335+
if _, err := r.Read(nil); err != (testError1{}) {
336+
t.Errorf("Read error: got %T, want testError1", err)
337+
}
338+
w.CloseWithError(testError2{})
339+
if _, err := r.Read(nil); err != (testError2{}) {
340+
t.Errorf("Read error: got %T, want testError2", err)
341+
}
342+
}
343+
319344
func TestPipeConcurrent(t *testing.T) {
320345
const (
321346
input = "0123456789abcdef"

0 commit comments

Comments
 (0)