Memory leak in Thumbnail generation

[mpldr]

When a timeout happens during thumbnail generation, the image.Paletted leaks, since the Goroutine has no way of terminating. My working hypothesis is that when the does not connect, the pipewriter is stuck and thus never completes. The solution would be to simply write the result to a channel. Since I plan to give some parts a bit of a make-over after the current misc-fixes PR, I can take a look at it afterwards.

pr, pw := io.Pipe()
go func(pw *io.PipeWriter, p *image.Paletted) {
	// we are stuck in this call
	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)

[mpldr]

Quick Update: Can confirm that the pipe was the issue. This quick and dirty fix has been running on my server for a few days now and even on the off-chance that a gif palette pops up, it disappears if a gc is triggered before retrieving the profile.

diff --git a/thumbnailing/i/gif.go b/thumbnailing/i/gif.go
index 676d0cb..625d88c 100644
--- a/thumbnailing/i/gif.go
+++ b/thumbnailing/i/gif.go
@@ -1,6 +1,8 @@
 package i
 
 import (
+	"fmt"
+	"bytes"
 	"errors"
 	"image"
 	"image/draw"
@@ -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("failed to encode static gif preview: %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("failed to encode animated gif preview: %w",err)
+	}
 
 	return &m.Thumbnail{
 		ContentType: "image/gif",
 		Animated:    true,
-		Reader:      pr,
+		Reader:      io.NopCloser(buf),
 	}, nil
 }

On that note, I'll probably add a removal of that profiler-header to the misc fixes PR, since I had to mess with my reverse proxy to run this like a normal person :D

go tool pprof -http=:6060 https://my-doma.in/_matrix/media/unstable/io.t2bot/debug/pprof/heap?gc