0

I have the following code of http handler which on subsequent request download the original image from Amazon S3 and convert it in the desired aspect ratio and save it back on s3. This code leaks memory and after sometime it crashes.I have handled everything from my side and also did the profiling on the code. But, still couldn't figure out the issue. Would appreciate if anyone can figure out here. FYI, I am using the go version go1.5.3 linux/amd64 version .

Output from profiling :

3259.27kB of 3302.42kB total (98.69%)
Dropped 258 nodes (cum <= 16.51kB)
Showing top 30 nodes out of 91 (cum >= 27.76kB)
      flat  flat%   sum%        cum   cum%
 1552.59kB 47.01% 47.01%  1552.59kB 47.01%  bytes.makeSlice
     584kB 17.68% 64.70%      584kB 17.68%  imagick._Cfunc_GoBytes
  257.38kB  7.79% 72.49%   257.38kB  7.79%  encoding/pem.Decode
  168.11kB  5.09% 77.58%   168.11kB  5.09%  crypto/tls.(*block).reserve
  165.09kB  5.00% 82.58%   389.49kB 11.79%  crypto/x509.parseCertificate
  105.32kB  3.19% 85.77%   105.32kB  3.19%  reflect.unsafe_NewArray
   83.64kB  2.53% 88.30%    83.64kB  2.53%  math/big.nat.make
   75.55kB  2.29% 90.59%    75.55kB  2.29%  net/http.(*Transport).dialConn
   64.02kB  1.94% 92.53%    64.02kB  1.94%  regexp.(*bitState).reset
   43.77kB  1.33% 93.85%    43.77kB  1.33%  crypto/x509.(*CertPool).AddCert
   40.44kB  1.22% 95.08%    40.44kB  1.22%  crypto/x509/pkix.(*Name).FillFromRDNSequence
   40.16kB  1.22% 96.29%    40.16kB  1.22%  encoding/asn1.parsePrintableString
   24.07kB  0.73% 97.02%    24.07kB  0.73%  net/http.newBufioWriterSize
   18.98kB  0.57% 97.60%    18.98kB  0.57%  net/http.newBufioReader
   16.14kB  0.49% 98.09%    64.77kB  1.96%  crypto/tls.(*Conn).readHandshake
   12.01kB  0.36% 98.45%   237.09kB  7.18%  encoding/asn1.parseField
    8.01kB  0.24% 98.69%    91.65kB  2.78%  crypto/x509.parsePublicKey
         0     0% 98.69%   112.33kB  3.40%  bufio.(*Reader).Read
         0     0% 98.69%    80.32kB  2.43%  bufio.(*Reader).fill
         0     0% 98.69%    27.76kB  0.84%  bufio.(*Writer).ReadFrom
         0     0% 98.69%    27.76kB  0.84%  bufio.(*Writer).flush
         0     0% 98.69%  1648.33kB 49.91%  bytes.(*Buffer).ReadFrom
         0     0% 98.69%    16.59kB   0.5%  bytes.(*Buffer).Write
         0     0% 98.69%    16.59kB   0.5%  bytes.(*Buffer).grow
         0     0% 98.69%   843.06kB 25.53%  crypto/tls.(*Conn).Handshake
         0     0% 98.69%   112.33kB  3.40%  crypto/tls.(*Conn).Read
         0     0% 98.69%    27.76kB  0.84%  crypto/tls.(*Conn).Write
         0     0% 98.69%   843.06kB 25.53%  crypto/tls.(*Conn).clientHandshake
         0     0% 98.69%   160.96kB  4.87%  crypto/tls.(*Conn).readRecord
         0     0% 98.69%    27.76kB  0.84%  crypto/tls.(*Conn).writeRecord

Code :

func main() {       

    imagick.Initialize() 
    defer imagick.Terminate()   

        myMux := http.NewServeMux()
        myMux.HandleFunc("/", serveHTTP)       

      if err := http.ListenAndServe(":8082", myMux); err != nil {
        logFatal("Error when starting or running http server: %v", err)
    }      

}

func serveHTTP(w http.ResponseWriter, r *http.Request) {

        var isMaster bool = true        
    var desiredAspectRatio float64 = 1.77   

    if r.RequestURI == "/favicon.ico" {
        w.WriteHeader(http.StatusNotFound)
        return
    }

    if len(strings.TrimSpace(r.URL.Query().Get("ar"))) != 0 {
        desiredAspectRatio, _ = strconv.ParseFloat(r.URL.Query().Get("ar"), 64)
    }

    if len(strings.TrimSpace(r.URL.Query().Get("ism"))) != 0 {      
        isMaster, _ = strconv.ParseBool(r.URL.Query().Get("ism"))
    }   

    imageUrl := strings.ToLower(r.URL.Path[1:])     

    isProcessed := CreateMaster(imageUrl, desiredAspectRatio, isMaster)     

    if isProcessed == false {
        w.WriteHeader(http.StatusNotFound) 
        return
    }

    if !sendResponse(w, r, imageUrl) {
        // w.WriteHeader() is skipped intentionally here, since the response may be already partially created.
        return
    }


    logRequestMessage(r, "SUCCESS")     
}


func sendResponse(w http.ResponseWriter, r *http.Request, imageUrl string) bool {

    w.Header().Set("Content-Type", "text/plain")        

    if _, err := w.Write([]byte("master created")); err != nil {
        logRequestError(r, "Cannot send image from imageUrl=%v to client: %v", imageUrl, err)
        return false
    }
    return true
}


func CreateMaster(keyName string, desiredAspectRatio float64, isMaster bool) bool {             

    s3Client := s3.New(session.New(), &aws.Config{Region: aws.String(region)})
        params := &s3.GetObjectInput{
        Bucket: aws.String(bucketName),
        Key: aws.String(keyName),
        }

    fmt.Println(" Master creation request for key : " + keyName)
    out, err := s3Client.GetObject(params)

    if err != nil { 
        return false                                       
    }

    defer out.Body.Close()  
    img, err := ioutil.ReadAll(out.Body)

    if err != nil { 
            return false      
    }                   

    mw := imagick.NewMagickWand()
    defer mw.Destroy()

    err = mw.ReadImageBlob(img)
    if err != nil {  
        return false                   
    }


    if isMaster == false {
        paramsPut := &s3.PutObjectInput{
                    Bucket:         aws.String(masterBucketName),
                    Key:            aws.String(keyName),
                    Body:         bytes.NewReader(mw.GetImageBlob()),
            }

        _, err = s3Client.PutObject(paramsPut)
        if err != nil {
            log.Printf("Couldn't put the image on s3 : " + keyName + "%s\n", err)       
        }

        return true
    }


        originalWidth := float64(mw.GetImageWidth())
        originalHeight := float64(mw.GetImageHeight())

    imageAspectRatio  := originalWidth / originalHeight
        masterWidth := cwMasterWidth
        masterHeight := cwMasterHeight
        masterAspectRatio := math.Trunc((cwMasterWidth / cwMasterHeight) * 100)/100

    if masterAspectRatio != desiredAspectRatio {           
                    masterAspectRatio = desiredAspectRatio
                }


    pwm := imagick.NewPixelWand()
    defer pwm.Destroy()

    tx := imagick.NewMagickWand()
    defer tx.Destroy()  

    if isMaster == true {               

            var w, h uint = 0, 0
            size := fmt.Sprintf("%dx%d^+0+0", w, h) 
                    if imageAspectRatio <= masterAspectRatio {                
                        // trim the height
            w = uint(originalWidth)
            h = (uint(originalWidth / masterAspectRatio))
               size = fmt.Sprintf("%dx%d^+0+0", w, h)
                     } else { 
                        // trim the width
            w = uint(originalHeight * masterAspectRatio)
            h = uint(originalHeight)
            size = fmt.Sprintf("%dx%d^+0+0", w, h)
                     }

            tx = mw.TransformImage("", size)        
            tx.SetImageGravity(imagick.GRAVITY_CENTER)
            offsetX := -(int(w) - int(tx.GetImageWidth())) / 2
            offsetY := -(int(h) - int(tx.GetImageHeight())) / 2
            err := tx.ExtentImage(w, h, offsetX, offsetY)

                    if float64(tx.GetImageWidth()) > masterWidth && float64(tx.GetImageHeight()) > masterHeight  {                                        
                        err = tx.ResizeImage(uint(masterWidth), uint(masterHeight), imagick.FILTER_BOX, 1)
            if err != nil {
                log.Printf("Inside CreateMaster function Couldn't resize the image : " + keyName + "%s\n", err) 
                return false                 
            }                                                           
                    }                                       
                }       

     paramsPut := &s3.PutObjectInput{
                    Bucket:         aws.String(masterBucketName),
                    Key:            aws.String(keyName),
                    Body:         bytes.NewReader(tx.GetImageBlob()),
            }

    _, err = s3Client.PutObject(paramsPut)
    if err != nil {
        log.Printf("Inside CreateMaster function Couldn't put the image on s3 : " + keyName + "%s\n", err)  
        return false        
    }

    return true
}
Naresh
  • 5,073
  • 12
  • 67
  • 124
  • I'm not familiar with `imagemagic` or `s3` API; but if `session` have anything to do with a user session; then you are not handling it correctly. Usually in-memory sessions use a map and they will stay in memory until you delete it explicitly (or have some sort of timeout policy implemented). – Kaveh Shahbazian Mar 03 '16 at 10:03
  • 1
    I'm not familiar with imagick either, but the call to `imagick.Terminate()` never executes. Maybe that's a clue. Other than that I skimmed over the code and it seems fine. – thwd Mar 03 '16 at 10:59
  • @thwd If, i don't include those lines then size of /tmp folder grows exponentially. That's why i have included them in my code – Naresh Mar 03 '16 at 11:37
  • 1
    @Naresh as I said, `imagick.Terminate()` is never executed. In consequence it can not have any effects. – thwd Mar 03 '16 at 12:09
  • `Terminate` does cleanup on exit, so it only needs to be called once. @Naresh: don't defer from `main`, as main will often be exiting via a signal handler, and defer won't get to execute. If you need to cleanup on exit, you need to create a signal handler. – JimB Mar 03 '16 at 15:07
  • Why are you creating a new Session and S3 for every request? I'm not certain it's the problem, (I think they will share the same `http.DefaultClient`), but there's no reason to do that since they are safe for concurrent use. – JimB Mar 03 '16 at 15:22
  • @JimB Well, If you read the comments on my other post then person who wrote the wrapper suggested me to include imagick.Initialize() defer imagick.Terminate() so that /tmp folder size doesn't increase. If, I don't include those two line there won't be any space left then. So what do i do then. http://stackoverflow.com/questions/35550850/lot-of-temp-magick-files-created-in‌​-temporary-folder. Moreover, thanking you for the input for the s3Client session. So, how do we create the singleton like object of session in go lang. So that i can use it again in my code – Naresh Mar 04 '16 at 10:17
  • @thwd Could you please explain the downvote. – Naresh Mar 04 '16 at 10:19
  • @KavehShahbazian I have changed and made the one object of Amazon s3 session. But still this doesn't help and still memory is increasing. – Naresh Mar 04 '16 at 13:32
  • @Naresh I haven't voted on this question. – thwd Mar 04 '16 at 13:51
  • @Naresh: I didn't say not to call Terminate, just that you should put it in signal handler because it won't execute when your program received signaled to exit. If fixing the s3 client doesn't help,then you need to provide more information and/or a reproducible test case. Does the Go heap continue to grow, or is the memory problem caused by allocations in C? What's the stack trace look like -- are you leaking goroutines somewhere? – JimB Mar 04 '16 at 15:01

1 Answers1

4

You are leaking a wand.

Here, you allocate a new wand and defer it to destroy:

tx := imagick.NewMagickWand()
defer tx.Destroy()  

But then further down, in an "if" block, you replace it with the wand that is returned from the call to TransformImage():

        tx = mw.TransformImage("", size)        
        tx.SetImageGravity(imagick.GRAVITY_CENTER)

If you completely get rid of the first allocation of that new magick wand, and simply make sure to Destroy() that new wand returned from TransformImage(), the leak goes away.

Ref the issue tracker, #72, for details

jdi
  • 90,542
  • 19
  • 167
  • 203