From 89d346bf511d9dd6503c3defaf0db64b8158c48d Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Sat, 1 Aug 2026 11:06:32 +0800 Subject: [PATCH] fix: return owned update download buffers Replace bytebufferpool-backed return values with bytes.Buffer storage so the downloaded and compressed slices remain valid after downloadBinary returns. Close the zstd encoder on copy failure and propagate final close errors. Add round-trip and pool-reuse regression coverage for both returned buffers. Co-authored-by: ChatGPT Co-authored-by: Claude --- cmd/update.go | 21 ++++++--------- cmd/update_test.go | 66 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 13 deletions(-) diff --git a/cmd/update.go b/cmd/update.go index 8232832c9..69847d719 100644 --- a/cmd/update.go +++ b/cmd/update.go @@ -19,6 +19,7 @@ package cmd import ( "bufio" + "bytes" "crypto" "crypto/tls" "encoding/hex" @@ -42,7 +43,6 @@ import ( xnet "github.com/minio/pkg/v3/net" "github.com/minio/selfupdate" gopsutilcpu "github.com/shirou/gopsutil/v3/cpu" - "github.com/valyala/bytebufferpool" ) const ( @@ -532,26 +532,21 @@ func downloadBinary(u *url.URL, mode string) (binCompressed []byte, bin []byte, } defer xhttp.DrainBody(reader) - b := bytebufferpool.Get() - bc := bytebufferpool.Get() - defer func() { - b.Reset() - bc.Reset() + var b, bc bytes.Buffer - bytebufferpool.Put(b) - bytebufferpool.Put(bc) - }() - - w, err := zstd.NewWriter(bc) + w, err := zstd.NewWriter(&bc) if err != nil { return nil, nil, err } - if _, err = io.Copy(w, io.TeeReader(reader, b)); err != nil { + if _, err = io.Copy(w, io.TeeReader(reader, &b)); err != nil { + _ = w.Close() return nil, nil, err } - w.Close() + if err = w.Close(); err != nil { + return nil, nil, err + } return bc.Bytes(), b.Bytes(), nil } diff --git a/cmd/update_test.go b/cmd/update_test.go index e1af9e38c..e94e049ea 100644 --- a/cmd/update_test.go +++ b/cmd/update_test.go @@ -18,6 +18,7 @@ package cmd import ( + "bytes" "encoding/hex" "fmt" "net/http" @@ -28,8 +29,73 @@ import ( "strings" "testing" "time" + + "github.com/klauspost/compress/zstd" + "github.com/valyala/bytebufferpool" ) +func TestDownloadBinaryReturnsOwnedBuffers(t *testing.T) { + previousMaxProcs := runtime.GOMAXPROCS(1) + t.Cleanup(func() { + runtime.GOMAXPROCS(previousMaxProcs) + }) + + payload := []byte("minio update payload") + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write(payload) + })) + t.Cleanup(server.Close) + + u, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + compressed, downloaded, err := downloadBinary(u, "server") + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(downloaded, payload) { + t.Fatalf("downloaded binary is %q, want %q", downloaded, payload) + } + decoder, err := zstd.NewReader(nil) + if err != nil { + t.Fatal(err) + } + t.Cleanup(decoder.Close) + decompressed, err := decoder.DecodeAll(compressed, nil) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(decompressed, payload) { + t.Fatalf("decompressed binary is %q, want %q", decompressed, payload) + } + wantCompressed := bytes.Clone(compressed) + wantDownloaded := bytes.Clone(downloaded) + + // Reuse buffers returned to bytebufferpool. downloadBinary's results must + // remain valid after the function returns, regardless of later pool users. + pooled := make([]*bytebufferpool.ByteBuffer, 8) + for i := range pooled { + pooled[i] = bytebufferpool.Get() + if cap(pooled[i].B) > 0 { + pooled[i].B = pooled[i].B[:cap(pooled[i].B)] + for j := range pooled[i].B { + pooled[i].B[j] = 0xa5 + } + } + } + for _, b := range pooled { + bytebufferpool.Put(b) + } + + if !bytes.Equal(compressed, wantCompressed) { + t.Fatal("compressed download aliases a buffer returned to bytebufferpool") + } + if !bytes.Equal(downloaded, wantDownloaded) { + t.Fatal("downloaded binary aliases a buffer returned to bytebufferpool") + } +} + func TestMinioVersionToReleaseTime(t *testing.T) { testCases := []struct { version string