From f132ff7702a6e4a02f78f86540856d96c7a8ffb3 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Tue, 28 Apr 2015 14:06:24 -0700 Subject: [PATCH] Use done channel to avoid goroutine leak This deals with a memory leak, caused by goroutines, experienced when using the s3 driver. Unfortunately, this section of the code leaks goroutines like a sieve. There is probably some refactoring that could be done to avoid this but instead, we have a done channel that will cause waiting goroutines to exit. Signed-off-by: Stephen J Day --- registry/storage/driver/s3/s3.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/registry/storage/driver/s3/s3.go b/registry/storage/driver/s3/s3.go index fe23262e..57871b5d 100644 --- a/registry/storage/driver/s3/s3.go +++ b/registry/storage/driver/s3/s3.go @@ -310,6 +310,7 @@ func (d *driver) WriteStream(path string, offset int64, reader io.Reader) (total var putErrChan chan error parts := []s3.Part{} var part s3.Part + done := make(chan struct{}) // stopgap to free up waiting goroutines multi, err := d.Bucket.InitMulti(d.s3Path(path), d.getContentType(), getPermissions(), d.getOptions()) if err != nil { @@ -344,6 +345,7 @@ func (d *driver) WriteStream(path string, offset int64, reader io.Reader) (total } d.putbuf(buf) // needs to be here to pick up new buf value + close(done) // free up any waiting goroutines }() // Fills from 0 to total from current @@ -407,7 +409,11 @@ func (d *driver) WriteStream(path string, offset int64, reader io.Reader) (total // the underlying s3 library should handle it, it doesn't seem to // be part of the shouldRetry function (see AdRoll/goamz/s3). defer func() { - putErrChan <- nil // for some reason, we do this no matter what. + select { + case putErrChan <- nil: // for some reason, we do this no matter what. + case <-done: + return // ensure we don't leak the goroutine + } }() if bytesRead <= 0 { @@ -449,7 +455,11 @@ func (d *driver) WriteStream(path string, offset int64, reader io.Reader) (total if err != nil { logrus.Errorf("error putting part, aborting: %v", err) - putErrChan <- err + select { + case putErrChan <- err: + case <-done: + return // don't leak the goroutine + } } // parts and partNumber are safe, because this function is the