From 00f52fd92dc603e8f006176aa4d05221e39a2f77 Mon Sep 17 00:00:00 2001 From: Moritz Poldrack Date: Mon, 4 Mar 2024 20:14:13 +0100 Subject: [PATCH] fix memory leak in gif thumbnail encoding When using an io.Pipe, it must be ensured that the pipe is drained. If it is not, all references required for the goroutine will not be freed by the garbage collector, as an active reference still exists (the abandoned goroutine the pipe is written to. To fix this, write to a non-blocking buffer. This may increase the RAM usage in the short run, but can be fully garbage collected, even if not read from. Since the size of the buffer is at most the size of the thumbnail and it being freed quickly, this should post a significantly smaller burden on the server. Signed-off-by: Moritz Poldrack Fixes: 4378a4e ("Avoid use of buffers throughout thumbnailing") Fixes: #561 --- thumbnailing/i/gif.go | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/thumbnailing/i/gif.go b/thumbnailing/i/gif.go index 676d0cba..7476175a 100644 --- a/thumbnailing/i/gif.go +++ b/thumbnailing/i/gif.go @@ -1,7 +1,9 @@ package i import ( + "bytes" "errors" + "fmt" "image" "image/draw" "image/gif" @@ -76,19 +78,15 @@ func (d gifGenerator) GenerateThumbnail(b io.Reader, contentType string, width i } // The thumbnailer decided that it shouldn't thumbnail, so encode it ourselves - pr, pw := io.Pipe() - go func(pw *io.PipeWriter, p *image.Paletted) { - err = u.Encode(ctx, pw, p) - if err != nil { - _ = pw.CloseWithError(errors.New("gif: error encoding still frame thumbnail: " + err.Error())) - } else { - _ = pw.Close() - } - }(pw, targetImg) + buf := bytes.NewBuffer(make([]byte, 0, 128*1024)) + err = u.Encode(ctx, buf, targetImg) + if err != nil { + return nil, fmt.Errorf("gif: error encoding static thumbnail: %w", err) + } return &m.Thumbnail{ Animated: false, ContentType: "image/png", - Reader: pr, + Reader: io.NopCloser(buf), }, nil } @@ -110,20 +108,16 @@ func (d gifGenerator) GenerateThumbnail(b io.Reader, contentType string, width i g.Config.Width = g.Image[0].Bounds().Max.X g.Config.Height = g.Image[0].Bounds().Max.Y - pr, pw := io.Pipe() - go func(pw *io.PipeWriter, g *gif.GIF) { - err = gif.EncodeAll(pw, g) - if err != nil { - _ = pw.CloseWithError(errors.New("gif: error encoding final thumbnail: " + err.Error())) - } else { - _ = pw.Close() - } - }(pw, g) + buf := bytes.NewBuffer(make([]byte, 0, 512*1024)) + err = gif.EncodeAll(buf, g) + if err != nil { + return nil, fmt.Errorf("gif: error encoding animated thumbnail: %w", err) + } return &m.Thumbnail{ ContentType: "image/gif", Animated: true, - Reader: pr, + Reader: io.NopCloser(buf), }, nil }