[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Using the ImageMagick backend seems to leak memory

From: Tassilo Horn
Subject: Re: Using the ImageMagick backend seems to leak memory
Date: Tue, 11 Jan 2011 20:10:29 +0100
User-agent: Gnus/5.110011 (No Gnus v0.11) Emacs/24.0.50 (gnu/linux)

address@hidden writes:

Hi Joakim,

> I've looked at this a little bit further.
> The error is that the image read with ReadImage is cloned by
> NewMagickWandFromImage not refered as I believed when I wrote the
> code.
> The code was written that way to avoid another memory problem I
> experienced with image bundles. If the current strategy remains,
> DestroyImage(im_image); should be the right fix.

I've tested that, and now memory is freed as expected.  Still, when I
used doc-view to view a 30 pages PDF (aka 30 large PNGs), the emacs
process started with ~11 MB when showing the first page and was at 12.2
MB after viewing all pages in sequence, killing the doc-view buffer, and
doing (clear-image-cache) and (garbage-collect).  But that might be no
real leak.

> However, the strategy should probably be changed to avoid the cloning.
> This will result in a more complex code path.

Well, you are the ImageMagick master. ;-)

Anyway, should I commit the following patch which includes your
DestroyImage() call that fixes the issue, and I additionally surrounded
image loading with calls to MagickWandGenesis()/MagickWandTerminus(),
because that's the way the API is used throughout the example codes in
the MagickWand API docs?

--8<---------------cut here---------------start------------->8---
=== modified file 'src/image.c'
--- src/image.c 2011-01-07 22:33:32 +0000
+++ src/image.c 2011-01-11 19:02:33 +0000
@@ -7521,6 +7521,9 @@
      image.  Interface :index is same as for GIF.  First we "ping" the
      image to see how many sub-images it contains. Pinging is faster
      than loading the image to find out things about it.  */
+  /* MagickWandGenesis() initializes the imagemagick library.  */
+  MagickWandGenesis ();
   image = image_spec_value (img->spec, QCindex, NULL);
   ino = INTEGERP (image) ? XFASTINT (image) : 0;
   ping_wand = NewMagickWand ();
@@ -7549,6 +7552,7 @@
   DestroyMagickWand (ping_wand);
   /* Now, after pinging, we know how many images are inside the
      file. If its not a bundle, just one.  */
@@ -7566,6 +7570,7 @@
       if (im_image != NULL)
          image_wand = NewMagickWandFromImage (im_image);
+          DestroyImage(im_image);
          status = MagickTrue;
@@ -7576,7 +7581,7 @@
       image_wand = NewMagickWand ();
       status = MagickReadImageBlob (image_wand, contents, size);
-  image_error ("im read failed", Qnil, Qnil);
   if (status == MagickFalse) goto imagemagick_error;
   /* If width and/or height is set in the display spec assume we want
@@ -7805,11 +7810,13 @@
   /* Final cleanup. image_wand should be the only resource left. */
   DestroyMagickWand (image_wand);
+  MagickWandTerminus ();
   return 1;
   DestroyMagickWand (image_wand);
+  MagickWandTerminus ();
   /* TODO more cleanup.  */
   image_error ("Error parsing IMAGEMAGICK image `%s'", img->spec, Qnil);
   return 0;
@@ -8681,8 +8688,6 @@
 #if defined (HAVE_IMAGEMAGICK)
   if (EQ (type, Qimagemagick))
-      /* MagickWandGenesis() initializes the imagemagick library.  */
-      MagickWandGenesis ();
       return CHECK_LIB_AVAILABLE (&imagemagick_type, 

--8<---------------cut here---------------end--------------->8---


reply via email to

[Prev in Thread] Current Thread [Next in Thread]