Skip to content

Commit 00f52fd

Browse files
committed
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 <[email protected]> Fixes: 4378a4e ("Avoid use of buffers throughout thumbnailing") Fixes: t2bot#561
1 parent 4be8c69 commit 00f52fd

File tree

1 file changed

+14
-20
lines changed

1 file changed

+14
-20
lines changed

thumbnailing/i/gif.go

+14-20
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package i
22

33
import (
4+
"bytes"
45
"errors"
6+
"fmt"
57
"image"
68
"image/draw"
79
"image/gif"
@@ -76,19 +78,15 @@ func (d gifGenerator) GenerateThumbnail(b io.Reader, contentType string, width i
7678
}
7779

7880
// The thumbnailer decided that it shouldn't thumbnail, so encode it ourselves
79-
pr, pw := io.Pipe()
80-
go func(pw *io.PipeWriter, p *image.Paletted) {
81-
err = u.Encode(ctx, pw, p)
82-
if err != nil {
83-
_ = pw.CloseWithError(errors.New("gif: error encoding still frame thumbnail: " + err.Error()))
84-
} else {
85-
_ = pw.Close()
86-
}
87-
}(pw, targetImg)
81+
buf := bytes.NewBuffer(make([]byte, 0, 128*1024))
82+
err = u.Encode(ctx, buf, targetImg)
83+
if err != nil {
84+
return nil, fmt.Errorf("gif: error encoding static thumbnail: %w", err)
85+
}
8886
return &m.Thumbnail{
8987
Animated: false,
9088
ContentType: "image/png",
91-
Reader: pr,
89+
Reader: io.NopCloser(buf),
9290
}, nil
9391
}
9492

@@ -110,20 +108,16 @@ func (d gifGenerator) GenerateThumbnail(b io.Reader, contentType string, width i
110108
g.Config.Width = g.Image[0].Bounds().Max.X
111109
g.Config.Height = g.Image[0].Bounds().Max.Y
112110

113-
pr, pw := io.Pipe()
114-
go func(pw *io.PipeWriter, g *gif.GIF) {
115-
err = gif.EncodeAll(pw, g)
116-
if err != nil {
117-
_ = pw.CloseWithError(errors.New("gif: error encoding final thumbnail: " + err.Error()))
118-
} else {
119-
_ = pw.Close()
120-
}
121-
}(pw, g)
111+
buf := bytes.NewBuffer(make([]byte, 0, 512*1024))
112+
err = gif.EncodeAll(buf, g)
113+
if err != nil {
114+
return nil, fmt.Errorf("gif: error encoding animated thumbnail: %w", err)
115+
}
122116

123117
return &m.Thumbnail{
124118
ContentType: "image/gif",
125119
Animated: true,
126-
Reader: pr,
120+
Reader: io.NopCloser(buf),
127121
}, nil
128122
}
129123

0 commit comments

Comments
 (0)