Cause of memory leak in annotate (reposted from wrong forum)

Post any defects you find in the released or beta versions of the ImageMagick software here. Include the ImageMagick version, OS, and any command-line required to reproduce the problem. Got a patch for a bug? Post it here.
Post Reply
edfhinton

Cause of memory leak in annotate (reposted from wrong forum)

Post by edfhinton »

(My apologies for the reposting of this over from the Developers forum. This being a bug, and one affecting me acutely now, I realized I posted in the wrong forum and am reposting here where bugs get attention. Not something I would normally do - sorry.)

I am using Magick++ and am experiencing substantial memory leaks calling annotate. I am using version 6.3.5-9 (Q16) and decided to trace through the code of annotate using a build for debug from source. What I found is that the code in AnnotateImage() has substantial leaks due to it doing the following:

1) On line 213, there is the following call:
annotate=CloneDrawInfo((ImageInfo *) NULL,draw_info);

looking at the code in CloneDrawInfo, it, among otherthings, uses CloneString to copy the ->text, ->geometry, and ->primitive members as well.

2) In the loop that starts on line 225 of annotate.c, there are a number of calls to CloneString to set some of the above members of annotate, particularly annotate->text. For example, on line 232, there is the code:
(void) CloneString(&annotate->text,textlist);

Looking at the code in CloneString, if the member being written is non-null, as wouyld be the case after the call to CloneDrawInfo on line 213, then the CloneString routine simply sets the pointer to NULL, (on line 221 of string.c) before going ahead and copying the source string.

This is odd, because earlier in the CloneString routine of string.c, it is seen that a call to CloneString with a null source will destroy the destinaiton existing string, but not if the source is non-null and will actually get cloned.

I suspect the same issue exists for more than just this member of the cloned drawInfo structure, partifularly the font member I believe may have a similar problem, but I haven't traced through every line yet.

Anyway, I am hoping this is something that can be resolved with a patch fairly soon because overall my app is eating about 800K per image I process just in the annotate calls (if I eliminate the annotate calls I do not lose the memory.) It only takes a few hundred images to compeltely crash the app due to memory problems.

I am happy to work off of patched source if a binary won;t be available anytime soon, but I am definitely wary of trying to deploy this to a customer (which i am supposed to do this week or next) as-is. This is for use in a hospital setting, so reliability is very important.

Thanks.
User avatar
magick
Site Admin
Posts: 11064
Joined: 2003-05-31T11:32:55-07:00

Re: Cause of memory leak in annotate (reposted from wrong forum)

Post by magick »

Excellent forensics. We have a fix for the memory leak you reported in ImageMagick 6.3.6-0 Beta available sometime tomorrow. 6.3.6-0 is scheduled for release within a week or two.
edfhinton

Re: Cause of memory leak in annotate (reposted from wrong forum)

Post by edfhinton »

FYI as a followup - rather than waiting for something to be released, I temporarily resolved this bug myself by adding the following two lines to string.c at line 221 just before *destination is set to NULL:

if (*destination != (char *) NULL)
*destination=DestroyString(*destination);

These are identical to the handling earlier in the routine in the scenario where the Source string is empty and the destination is essentially being cleared out.

After making that change, I was able to do an overnight processing run of almost 4000 images, placing around 65 lines of annotations on each image, with no net loss of memory.

I suspect this accounts for memory losses in more places than just annotate(), because prior to the change, if I commented out the calls to annotate(), I still had some small leaks in other calls to ImageMagick.

I look forward to an actual release that resolves this.
Post Reply