Revert "io: allocate copy buffers from a pool"

This reverts CL 456555.

Reason for revert: This seems too likely to exercise race conditions
in code where a Write call continues to access its buffer after
returning. The HTTP/2 ResponseWriter is one such example.

Reverting this change while we think about this some more.

For #57202

Change-Id: Ic86823f81d7da410ea6b3f17fb5b3f9a979e3340
Reviewed-on: https://go-review.googlesource.com/c/go/+/467095
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This commit is contained in:
Damien Neil 2023-02-09 14:24:46 -08:00
parent 6e5c26084f
commit b02d5d325a
2 changed files with 32 additions and 15 deletions

View File

@ -400,13 +400,6 @@ func CopyBuffer(dst Writer, src Reader, buf []byte) (written int64, err error) {
return copyBuffer(dst, src, buf)
}
var bufPool = sync.Pool{
New: func() any {
b := make([]byte, 32*1024)
return &b
},
}
// copyBuffer is the actual implementation of Copy and CopyBuffer.
// if buf is nil, one is allocated.
func copyBuffer(dst Writer, src Reader, buf []byte) (written int64, err error) {
@ -420,9 +413,15 @@ func copyBuffer(dst Writer, src Reader, buf []byte) (written int64, err error) {
return rt.ReadFrom(src)
}
if buf == nil {
bufp := bufPool.Get().(*[]byte)
defer bufPool.Put(bufp)
buf = *bufp
size := 32 * 1024
if l, ok := src.(*LimitedReader); ok && int64(size) > l.N {
if l.N < 1 {
size = 1
} else {
size = int(l.N)
}
}
buf = make([]byte, size)
}
for {
nr, er := src.Read(buf)
@ -638,14 +637,21 @@ func (discard) WriteString(s string) (int, error) {
return len(s), nil
}
var blackHolePool = sync.Pool{
New: func() any {
b := make([]byte, 8192)
return &b
},
}
func (discard) ReadFrom(r Reader) (n int64, err error) {
bufp := bufPool.Get().(*[]byte)
bufp := blackHolePool.Get().(*[]byte)
readSize := 0
for {
readSize, err = r.Read(*bufp)
n += int64(readSize)
if err != nil {
bufPool.Put(bufp)
blackHolePool.Put(bufp)
if err == EOF {
return n, nil
}

View File

@ -567,12 +567,16 @@ type writerOnly struct {
// to a *net.TCPConn with sendfile, or from a supported src type such
// as a *net.TCPConn on Linux with splice.
func (w *response) ReadFrom(src io.Reader) (n int64, err error) {
bufp := copyBufPool.Get().(*[]byte)
buf := *bufp
defer copyBufPool.Put(bufp)
// Our underlying w.conn.rwc is usually a *TCPConn (with its
// own ReadFrom method). If not, just fall back to the normal
// copy method.
rf, ok := w.conn.rwc.(io.ReaderFrom)
if !ok {
return io.Copy(writerOnly{w}, src)
return io.CopyBuffer(writerOnly{w}, src, buf)
}
// Copy the first sniffLen bytes before switching to ReadFrom.
@ -580,7 +584,7 @@ func (w *response) ReadFrom(src io.Reader) (n int64, err error) {
// source is available (see golang.org/issue/5660) and provides
// enough bytes to perform Content-Type sniffing when required.
if !w.cw.wroteHeader {
n0, err := io.Copy(writerOnly{w}, io.LimitReader(src, sniffLen))
n0, err := io.CopyBuffer(writerOnly{w}, io.LimitReader(src, sniffLen), buf)
n += n0
if err != nil || n0 < sniffLen {
return n, err
@ -598,7 +602,7 @@ func (w *response) ReadFrom(src io.Reader) (n int64, err error) {
return n, err
}
n0, err := io.Copy(writerOnly{w}, src)
n0, err := io.CopyBuffer(writerOnly{w}, src, buf)
n += n0
return n, err
}
@ -795,6 +799,13 @@ var (
bufioWriter4kPool sync.Pool
)
var copyBufPool = sync.Pool{
New: func() any {
b := make([]byte, 32*1024)
return &b
},
}
func bufioWriterPool(size int) *sync.Pool {
switch size {
case 2 << 10: