proposal_io_buffered_pipe.md

Proposal: io: add a buffered pipe

Summary

Add a way to easily create a buffered io.Pipe. Currently, one has to write their own from scratch. This leads to dozens of separate implementations in the wild, and likely many correctness issues and hidden bugs.

Description

io.Pipe is synchronous, as per https://golang.org/pkg/io/#Pipe:

Reads and Writes on the pipe are matched one to one except when multiple Reads are needed to consume a single Write. That is, each Write to the PipeWriter blocks until it has satisfied one or more Reads from the PipeReader that fully consume the written data. The data is copied directly from the Write to the corresponding Read (or Reads); there is no internal buffering.

This is fine for most use cases. However, a buffered pipe can still be useful or necessary in many situations. Below is a list of third-party implementations I was able to find after a five-minute search:

• https://github.com/golang/go/blob/0436b162397018c45068b47ca1b5924a3eafdee0/src/net/net_fake.go#L173
• https://github.com/golang/go/blob/f58b02a29c395b5ec28bb548b1fd3ca18e2b3a4f/src/net/http/h2_bundle.go#L3688
• https://github.com/golang/crypto/blob/a49355c7e3f8fe157a85be2f77e6e269a0f89602/ssh/handshake_test.go#L42
• https://github.com/moby/vpnkit/blob/237a0c3dfd57753f798a4cb4143cb1ed2ee118b9/go/pkg/libproxy/loopbackconn.go
• https://github.com/vmware/vic/blob/68eef3661c332fad0fdf71b99d4a10bda06ee1e4/lib/install/vchlog/buffered_pipe.go
• https://github.com/v2ray/v2ray-core/blob/891bdd14fe7cbb99b3d64894a633e83cd2691b72/transport/pipe/impl.go
• https://github.com/vanadium/go.lib/blob/d0dec8edd1bfe451a41763608a8518f1dba8e7ae/gosh/buffered_pipe.go
• https://github.com/karalabe/bufioprop/blob/98d36783a81dc759efaef68b4ad9a7d3bfa4090b/pipe.go
• https://github.com/kr/spdy/blob/b6b3b6ca43a79eafe2fb96a44066e27768457928/spdyframing/pipe.go#L7
• https://github.com/whyrusleeping/bufpipe

Note the first two, which live in the standard library itself. They're also interesting, because net's requires deadlines, and http's supports any underlying pipe buffer implementation.

The main point of the proposal is that there are dozens of well-used buffered pipe implementations out there. This is particularly worrying, because writing a correct one isn't easy. It's very likely that some of the implementations out there have subtle bugs, such as misusing sync.Cond or being racy when many reads and writes happen concurrently.

I'm raising this proposal precisely because I had written my own buggy buffered pipe, which made go test -race find data races about 5% of the time. I "fixed" that by rewriting my tests to work around the lack of buffering, and using io.Pipe instead: https://github.com/mvdan/sh/commit/5ffc4d94f4d4e57cbb02063768b67c1b337bf9cc

Granted, my code being incorrect doesn't mean everyone's code is too. But it does help illustrate the point; writing a buffered pipe seems easy, and the lack of an implementation in the standard library seems to encourage that people write their own, likely buggy implementation.

This has been discussed in golang-dev before, although no common implementation was found or proposed: https://groups.google.com/forum/#!topic/golang-dev/k0bSal8eDyE

There's also a proposal to make net.Pipe asynchronous in Go2; golang/go#24205. That proposal is somewhat related to this one, since net.Pipe currently uses io.Pipe under the hood. This could however be changed in Go2.

Proposed APIs

The simplest change would be to add something like:

// BufferedPipe creates an asynchronous in-memory pipe, internally buffering up
// to n bytes.
func BufferedPipe(size int) (*PipeReader, *PipeWriter)

Ideally PipeReader and PipeWriter would have been interface types from the start. Within Go1, we can work around that by having the PipeWriter struct simply embed the actual pipe implementation, be it synchronous or asynchronous.

Another possible path in Go1 is to go the http2 route, allowing any underlying pipe buffer implementation. I personally think this would be a bit too niche for the io package, but perhaps @bradfitz has opinions on that.

If the net package needs read and write deadlines, they could always be added to the reader and writer returned by the constructors. This should be a backwards compatible change, and could be discussed as a separate proposal.

Finally, if we were to consider this as a Go2 change, the cleaned up version of my first proposed change would be:

type PipeReader interface {
	io.Reader
	io.Closer
	CloseWithError(err error) error
}

type PipeWriter interface {
	io.Writer
	io.Closer
	CloseWithError(err error) error
}

func Pipe() (PipeReader, PipeWriter)
func BufferedPipe(size int) (PipeReader, PipeWriter)

We could even go one step further and mirror make(chan T, size), only having func Pipe(size int) where Pipe(0) would return a synchronous/unbuffered pipe.

The net package could either mirror these proposed Go2 changes, or break with the connection to the io package entirely.