]> Cypherpunks repositories - gostls13.git/commitdiff
archive/zip: reduce memory held by Writer.Copy
authorJonathan Amsterdam <jba@google.com>
Sat, 3 Feb 2024 16:48:50 +0000 (11:48 -0500)
committerJonathan Amsterdam <jba@google.com>
Tue, 6 Feb 2024 12:43:52 +0000 (12:43 +0000)
Make a copy of the argument File's FileHeader, and pass a pointer
to the copy to CreateRaw.

Passing the pointer directly causes the entire `File` to be referenced
by the receiver. The `File` includes a reference to the `ReaderAt`
underlying the `Reader`, so all its memory, which may be the entire
contents of the archive, is prevented from being garbage-collected.

Also, explain the issue in the doc comment for CreateRaw. We
cannot change its behavior because someone may depend on the
preserving the identity of its argument pointer.

For #65499.

Change-Id: Ieb4963a0ea30539d597547d3511accbd8c6b5c5a
Reviewed-on: https://go-review.googlesource.com/c/go/+/560238
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
src/archive/zip/writer.go

index e33df2431c72497913c6f3758085ddaf80d70af5..9e2dcff7137671ce2dd1c164a2237bee8f87c897 100644 (file)
@@ -433,6 +433,10 @@ func writeHeader(w io.Writer, h *header) error {
 // [Writer.CreateHeader], [Writer.CreateRaw], or [Writer.Close].
 //
 // In contrast to [Writer.CreateHeader], the bytes passed to Writer are not compressed.
+//
+// CreateRaw's argument is stored in w. If the argument is a pointer to the embedded
+// [FileHeader] in a [File] obtained from a [Reader] created from in-memory data,
+// then w will refer to all of that memory.
 func (w *Writer) CreateRaw(fh *FileHeader) (io.Writer, error) {
        if err := w.prepare(fh); err != nil {
                return nil, err
@@ -471,7 +475,10 @@ func (w *Writer) Copy(f *File) error {
        if err != nil {
                return err
        }
-       fw, err := w.CreateRaw(&f.FileHeader)
+       // Copy the FileHeader so w doesn't store a pointer to the data
+       // of f's entire archive. See #65499.
+       fh := f.FileHeader
+       fw, err := w.CreateRaw(&fh)
        if err != nil {
                return err
        }