Skip to content

Commit b02d5d3

Browse files
committed
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 <[email protected]> Run-TryBot: Damien Neil <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
1 parent 6e5c260 commit b02d5d3

File tree

2 files changed

+32
-15
lines changed

2 files changed

+32
-15
lines changed

src/io/io.go

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -400,13 +400,6 @@ func CopyBuffer(dst Writer, src Reader, buf []byte) (written int64, err error) {
400400
return copyBuffer(dst, src, buf)
401401
}
402402

403-
var bufPool = sync.Pool{
404-
New: func() any {
405-
b := make([]byte, 32*1024)
406-
return &b
407-
},
408-
}
409-
410403
// copyBuffer is the actual implementation of Copy and CopyBuffer.
411404
// if buf is nil, one is allocated.
412405
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) {
420413
return rt.ReadFrom(src)
421414
}
422415
if buf == nil {
423-
bufp := bufPool.Get().(*[]byte)
424-
defer bufPool.Put(bufp)
425-
buf = *bufp
416+
size := 32 * 1024
417+
if l, ok := src.(*LimitedReader); ok && int64(size) > l.N {
418+
if l.N < 1 {
419+
size = 1
420+
} else {
421+
size = int(l.N)
422+
}
423+
}
424+
buf = make([]byte, size)
426425
}
427426
for {
428427
nr, er := src.Read(buf)
@@ -638,14 +637,21 @@ func (discard) WriteString(s string) (int, error) {
638637
return len(s), nil
639638
}
640639

640+
var blackHolePool = sync.Pool{
641+
New: func() any {
642+
b := make([]byte, 8192)
643+
return &b
644+
},
645+
}
646+
641647
func (discard) ReadFrom(r Reader) (n int64, err error) {
642-
bufp := bufPool.Get().(*[]byte)
648+
bufp := blackHolePool.Get().(*[]byte)
643649
readSize := 0
644650
for {
645651
readSize, err = r.Read(*bufp)
646652
n += int64(readSize)
647653
if err != nil {
648-
bufPool.Put(bufp)
654+
blackHolePool.Put(bufp)
649655
if err == EOF {
650656
return n, nil
651657
}

src/net/http/server.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -567,20 +567,24 @@ type writerOnly struct {
567567
// to a *net.TCPConn with sendfile, or from a supported src type such
568568
// as a *net.TCPConn on Linux with splice.
569569
func (w *response) ReadFrom(src io.Reader) (n int64, err error) {
570+
bufp := copyBufPool.Get().(*[]byte)
571+
buf := *bufp
572+
defer copyBufPool.Put(bufp)
573+
570574
// Our underlying w.conn.rwc is usually a *TCPConn (with its
571575
// own ReadFrom method). If not, just fall back to the normal
572576
// copy method.
573577
rf, ok := w.conn.rwc.(io.ReaderFrom)
574578
if !ok {
575-
return io.Copy(writerOnly{w}, src)
579+
return io.CopyBuffer(writerOnly{w}, src, buf)
576580
}
577581

578582
// Copy the first sniffLen bytes before switching to ReadFrom.
579583
// This ensures we don't start writing the response before the
580584
// source is available (see golang.org/issue/5660) and provides
581585
// enough bytes to perform Content-Type sniffing when required.
582586
if !w.cw.wroteHeader {
583-
n0, err := io.Copy(writerOnly{w}, io.LimitReader(src, sniffLen))
587+
n0, err := io.CopyBuffer(writerOnly{w}, io.LimitReader(src, sniffLen), buf)
584588
n += n0
585589
if err != nil || n0 < sniffLen {
586590
return n, err
@@ -598,7 +602,7 @@ func (w *response) ReadFrom(src io.Reader) (n int64, err error) {
598602
return n, err
599603
}
600604

601-
n0, err := io.Copy(writerOnly{w}, src)
605+
n0, err := io.CopyBuffer(writerOnly{w}, src, buf)
602606
n += n0
603607
return n, err
604608
}
@@ -795,6 +799,13 @@ var (
795799
bufioWriter4kPool sync.Pool
796800
)
797801

802+
var copyBufPool = sync.Pool{
803+
New: func() any {
804+
b := make([]byte, 32*1024)
805+
return &b
806+
},
807+
}
808+
798809
func bufioWriterPool(size int) *sync.Pool {
799810
switch size {
800811
case 2 << 10:

0 commit comments

Comments
 (0)