Memory issue with the imagick initialization #72
Comments
|
The call to Would you be able to reduce your code into a smaller example that would enable us to easily reproduce the memory leak? |
|
I use these bindings as part of a web application, I don't believe I call Initialize/Terminate. The process runs forever, when would be the appropriate time to call these functions? |
|
@justinfx If let' s say what your are saying is correct then memory leak should be in CreateMaster function. So, i have pasted the code here. For more information you can refer to the SO link. |
|
@xealot , ideally you would call them in your |
|
@palaiyacw I appreciate the paste. However a runnable repro would have been better. But I can just try to repro the iMagick operations you are doing here and see if I can find a leak. Since iMagick is a thin binding, technically I should only have to look as far as the iMagick methods that you call to see if we overlooked releasing temp resources while calling into C. |
|
@justinfx I would appreciate that. If you can see whether temp resources are being release or not in the iMagick method and please reproduce the operation according to your suitability |
|
I took a look and didn't see anything stand out in the Imagick layer that would cause the leak. The wrapper functions are very very small, and I see cleanups for any allocated C memory. I tried to reproduce it using your example, and all the http and S3 stuff removed. Didn't see a leak. Can you take this code and try to make it leak? package main
import (
"fmt"
"io/ioutil"
"log"
"os"
"runtime"
"gopkg.in/gographics/imagick.v2/imagick"
)
const SrcImage = `/path/to/source/image.jpg`
func main() {
imagick.Initialize()
defer imagick.Terminate()
i := 0
var stats runtime.MemStats
for {
Process()
runtime.ReadMemStats(&stats)
i++
fmt.Printf("After run #%04d: Alloc: %v, Idle: %v, InUse: %v, Objects: %v\n", i,
stats.HeapAlloc, stats.HeapIdle, stats.HeapInuse, stats.HeapObjects)
}
}
func Process() {
f, err := os.Open(SrcImage)
if err != nil {
log.Fatalf("Failed to open source image: %v", err)
}
defer f.Close()
img, err := ioutil.ReadAll(f)
if err != nil {
log.Fatalf("Failed to read image file: %v", err)
}
mw := imagick.NewMagickWand()
defer mw.Destroy()
err = mw.ReadImageBlob(img)
if err != nil {
log.Fatalf("Failed to read image blob: %v", err)
}
originalWidth := float64(mw.GetImageWidth())
originalHeight := float64(mw.GetImageHeight())
imageAspectRatio := originalWidth / originalHeight
masterWidth := 320.0
masterHeight := 180.0
masterAspectRatio := masterWidth / masterHeight
pwm := imagick.NewPixelWand()
defer pwm.Destroy()
tx := imagick.NewMagickWand()
defer tx.Destroy()
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 err != nil {
log.Fatalf("Failed calling ExtendImage(%v, %v, %v, %v): %v",
w, h, offsetX, offsetY, err)
}
if float64(tx.GetImageWidth()) > masterWidth && float64(tx.GetImageHeight()) > masterHeight {
err = tx.ResizeImage(uint(masterWidth), uint(masterHeight), imagick.FILTER_BOX, 1)
if err != nil {
log.Fatalf("Inside CreateMaster function Couldn't "+
"resize the image %s: %s", SrcImage, err)
}
}
} |
|
@justinfx Actually even i also find difficult to replicate the same issue while testing the code locally. I tested your code and couldn't find anything. But I am attaching the screen shot of my I am attaching the syslog as well as screen shot of
Just one more point i want to raise here is that i keep all images in the memory and resize them in memory and write it to the client. So, could it be the related to GC. |
|
The temp file stuff is going to be more related to ImageMagick directly, so you may want to follow up with the maintainers or the ImageMagick community. Out of curiosity, does the call to If we can't get a local reproduction working, then I am not too sure what to do from here. For all I know, ImageMagick is still hanging onto memory until Terminate is called. I also had a long running Image Cache Server running with imagick. It would serve cached blobs out of an LRU, and when a request image doesn't exist, it would read it, transform it, and cache it. I don't recall running into this problem. But I have since migrated to using my other bindings, for OpenImageIO: https://github.com/justinfx/openimageigo |
|
Here is another question... if you completely disable the S3 logic in your code, and always perform the image processing and serving, does it still have the memory issues? I want to remove the possibility of the leak happening somewhere in the S3 area. |
|
@justinfx I have tested your code on amazon ec2 machine and I can see the memory footprint increasing constantly. Here are the screen shots of the same. EC2 machine OS info:
|
|
If you run it with What version of ImageMagick is this? Which branch of imagick are you using? |
|
I have tested with Output of the program : |
|
What about the output with gotrace=1? GODEBUG=gctrace=1 |
|
After run #1: Alloc: 2170240, Idle: 483328, InUse: 2367488, Objects: 196 |
|
@justinfx You need to make it run for atleast 1000 run. You will then notice memory footprint increase gradually. |
|
I've found the issue. You have a bug in your code and 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, you replace it with the wand that is returned from the call to 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 This happens because the current design of imagick is for the user to make sure they free all of their resources explicitly, just like files and sockets. There is a pending pull request, #62, which adds a finalizer to objects, so that they will always get destroyed once they are garbage collected. Can you test and confirm? |
|
@justinfx Thanking you so much. It finally fixed the leak from my code. But i couldn't understand i am just assigning a new value in the |
|
The arguments to the defer statement are evaluated when the defer executes. Not when the function is evaluated. You were scheduling the original object to destroy and not the new one. Either way, regardless of whether it applies when the defer evaluates or later, you leak an object. It just happens, because of the language spec, that you leak the larger one (the later one that sucks up the memory) |
|
Also my method of debuging was to comment out most the code and test, then observe the results and slowly uncomment more code until I could pinpoint when a leak was observable. It wasn't until I introduced the tx logic that I saw the leak. And when I stared more closely at it, I ended up seeing the reassignment |
I have explained the code in the following link. Which uses the
to initialize the library. If, I don't include these lines in the code my /tmp folder size keep on increasing. Where as on the other hand if i include them in code my RAM size increases monotonously and in the end my handler crashes. Kindly look into this matter. Why is it happening. Moreover, as such i don't have any memory leak in my code.
http://stackoverflow.com/questions/35768266/memory-leak-in-go-lang-http-handler
The text was updated successfully, but these errors were encountered: