code review 5708056: io: Pipes and ReadAt are safe to use concurrently.

io: Pipes and ReadAt are safe to use concurrently.

Updates issue 1599.

[r, 2012-02-29 02:38:06]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go/

[bradfitz, 2012-02-29 02:43:29]

Want to include io.ReaderAt in this CL too? It says as such now,
implicitly, but consistent wording would be nice considering the new
warning at top.
 On Feb 29, 2012 1:38 PM, <r@golang.org> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> io: Pipes are safe to use concurrently.
>
> Updates issue 1599.
>
> Please review this at
http://codereview.appspot.com/**5708056/<http://codereview.appspot.com/5708056/>
>
> Affected files:
>  M src/pkg/io/pipe.go
>
>
> Index: src/pkg/io/pipe.go
> ==============================**==============================**=======
> --- a/src/pkg/io/pipe.go
> +++ b/src/pkg/io/pipe.go
> @@ -175,6 +175,10 @@
>  // with code expecting an io.Writer.
>  // Reads on one end are matched with writes on the other,
>  // copying data directly between the two; there is no internal buffering.
> +// It is safe to call Read and Write in parallel with each other or with
> +// Close. Close will complete once pending I/O is done. Parallel calls to
> +// Read, and parallel calls to Write, are also safe:
> +// the invidual calls will be gated sequentially.
>  func Pipe() (*PipeReader, *PipeWriter) {
>        p := new(pipe)
>        p.rwait.L = &p.l
>
>
>

[bradfitz, 2012-02-29 02:43:29]

LGTM
 On Feb 29, 2012 1:38 PM, <r@golang.org> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> io: Pipes are safe to use concurrently.
>
> Updates issue 1599.
>
> Please review this at
http://codereview.appspot.com/**5708056/<http://codereview.appspot.com/5708056/>
>
> Affected files:
>  M src/pkg/io/pipe.go
>
>
> Index: src/pkg/io/pipe.go
> ==============================**==============================**=======
> --- a/src/pkg/io/pipe.go
> +++ b/src/pkg/io/pipe.go
> @@ -175,6 +175,10 @@
>  // with code expecting an io.Writer.
>  // Reads on one end are matched with writes on the other,
>  // copying data directly between the two; there is no internal buffering.
> +// It is safe to call Read and Write in parallel with each other or with
> +// Close. Close will complete once pending I/O is done. Parallel calls to
> +// Read, and parallel calls to Write, are also safe:
> +// the invidual calls will be gated sequentially.
>  func Pipe() (*PipeReader, *PipeWriter) {
>        p := new(pipe)
>        p.rwait.L = &p.l
>
>
>

[r, 2012-02-29 03:08:00]

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

Please take another look.

[bradfitz, 2012-02-29 08:54:58]

LGTM

[rsc, 2012-02-29 16:36:37]

LGTM

http://codereview.appspot.com/5708056/diff/2002/src/pkg/io/io.go
File src/pkg/io/io.go (right):

http://codereview.appspot.com/5708056/diff/2002/src/pkg/io/io.go#newcode164
src/pkg/io/io.go:164: // Clients of ReadAt should be able to execute parallel
ReadAt
s/of ReadAt should be able to/can/

[r2, 2012-03-01 00:23:47]

On Mar 1, 2012, at 3:36 AM, rsc@golang.org wrote:

> LGTM
> 
> 
> 
> http://codereview.appspot.com/5708056/diff/2002/src/pkg/io/io.go
> File src/pkg/io/io.go (right):
> 
> http://codereview.appspot.com/5708056/diff/2002/src/pkg/io/io.go#newcode164
> src/pkg/io/io.go:164: // Clients of ReadAt should be able to execute
> parallel ReadAt
> s/of ReadAt should be able to/can/

ok, although the 'should' was really directed at the implementers rather than
the users.

[r, 2012-03-01 00:24:19]

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

io: Pipes and ReadAt are safe to use concurrently.

Updates issue 1599.

R=golang-dev, bradfitz, rsc, r
CC=golang-dev
http://codereview.appspot.com/5708056