code review 994043: pipe: implementation #3; this time for sure!

pipe: implementation #3; this time for sure!
Added goroutine; got simpler.

Fixes deadlock when doing Read+Close
or Write+Close on same end.

[rsc, 2010-04-27 09:06:09]

Hello r (cc: golang-dev@googlegroups.com),

I'd like you to review this change.

[cw, 2010-04-27 09:25:07]

curious

http://codereview.appspot.com/994043/diff/2001/3001
File src/pkg/io/pipe.go (right):

http://codereview.appspot.com/994043/diff/2001/3001#newcode279
src/pkg/io/pipe.go:279: // but 6g doesn't see the finalizer method.
is that a gc bug?

[r, 2010-04-27 16:44:17]

LGTM

nicer

http://codereview.appspot.com/994043/diff/2001/3001
File src/pkg/io/pipe.go (right):

http://codereview.appspot.com/994043/diff/2001/3001#newcode31
src/pkg/io/pipe.go:31: cdone chan int // read or write half is done
why all the c's?

http://codereview.appspot.com/994043/diff/2001/3001#newcode46
src/pkg/io/pipe.go:46: // Read and Write are enabled to start with.
s/to start with/at the start/

http://codereview.appspot.com/994043/diff/2001/3001#newcode214
src/pkg/io/pipe.go:214: // to the write half of the pipe will return the error
rerr.
i would s/rerr/err/ here and s/werr/err/ in the write case

http://codereview.appspot.com/994043/diff/2001/3002
File src/pkg/io/pipe_test.go (right):

http://codereview.appspot.com/994043/diff/2001/3002#newcode221
src/pkg/io/pipe_test.go:221: t.Errorf("read on closed pipe returned %d", n)
print the error too?

[rsc, 2010-04-27 17:11:28]

> http://codereview.appspot.com/994043/diff/2001/3001#newcode279
> src/pkg/io/pipe.go:279: // but 6g doesn't see the finalizer method.
> is that a gc bug?

yes

[rsc, 2010-04-27 17:16:51]

done.

On Tue, Apr 27, 2010 at 09:44,  <r@golang.org> wrote:
> LGTM
>
> nicer
>
>
>
> http://codereview.appspot.com/994043/diff/2001/3001
> File src/pkg/io/pipe.go (right):
>
> http://codereview.appspot.com/994043/diff/2001/3001#newcode31
> src/pkg/io/pipe.go:31: cdone chan int // read or write half is done
> why all the c's?
>
> http://codereview.appspot.com/994043/diff/2001/3001#newcode46
> src/pkg/io/pipe.go:46: // Read and Write are enabled to start with.
> s/to start with/at the start/
>
> http://codereview.appspot.com/994043/diff/2001/3001#newcode214
> src/pkg/io/pipe.go:214: // to the write half of the pipe will return the
> error rerr.
> i would s/rerr/err/ here and s/werr/err/ in the write case
>
> http://codereview.appspot.com/994043/diff/2001/3002
> File src/pkg/io/pipe_test.go (right):
>
> http://codereview.appspot.com/994043/diff/2001/3002#newcode221
> src/pkg/io/pipe_test.go:221: t.Errorf("read on closed pipe returned %d",
> n)
> print the error too?
>
> http://codereview.appspot.com/994043/show
>

[rsc, 2010-04-27 17:17:24]

*** Submitted as http://code.google.com/p/go/source/detail?r=76161e97acf3 ***

pipe: implementation #3; this time for sure!
Added goroutine; got simpler.

Fixes deadlock when doing Read+Close
or Write+Close on same end.

R=r, cw
CC=golang-dev
http://codereview.appspot.com/994043