From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Tassilo Horn Newsgroups: gmane.emacs.devel Subject: Re: Using the ImageMagick backend seems to leak memory Date: Tue, 11 Jan 2011 20:10:29 +0100 Message-ID: <8739oz8dei.fsf@member.fsf.org> References: <87sjx6zczl.fsf@member.fsf.org> <30611405.post@talk.nabble.com> <874o9l86eb.fsf@member.fsf.org> <87zkr7nxo8.fsf@member.fsf.org> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: dough.gmane.org 1294773057 4857 80.91.229.12 (11 Jan 2011 19:10:57 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Tue, 11 Jan 2011 19:10:57 +0000 (UTC) Cc: Miles Bader , Jashy , Emacs-devel@gnu.org To: joakim@verona.se Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue Jan 11 20:10:50 2011 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1PcjcH-0003Vi-GX for ged-emacs-devel@m.gmane.org; Tue, 11 Jan 2011 20:10:49 +0100 Original-Received: from localhost ([127.0.0.1]:59344 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PcjcG-000199-SM for ged-emacs-devel@m.gmane.org; Tue, 11 Jan 2011 14:10:48 -0500 Original-Received: from [140.186.70.92] (port=47818 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Pcjc7-00014p-92 for Emacs-devel@gnu.org; Tue, 11 Jan 2011 14:10:41 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Pcjc0-0003ed-Lu for Emacs-devel@gnu.org; Tue, 11 Jan 2011 14:10:39 -0500 Original-Received: from out1.smtp.messagingengine.com ([66.111.4.25]:39303) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Pcjc0-0003eQ-I9; Tue, 11 Jan 2011 14:10:32 -0500 Original-Received: from compute2.internal (compute2.nyi.mail.srv.osa [10.202.2.42]) by gateway1.messagingengine.com (Postfix) with ESMTP id DE5EB202D1; Tue, 11 Jan 2011 14:10:31 -0500 (EST) Original-Received: from frontend1.messagingengine.com ([10.202.2.160]) by compute2.internal (MEProxy); Tue, 11 Jan 2011 14:10:31 -0500 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=messagingengine.com; h=from:to:cc:subject:references:date:in-reply-to:message-id:mime-version:content-type; s=smtpout; bh=nMQMQRJB/+nzZiIlMH7Iy+LmaEg=; b=tJy1BGIW3S2LNBGuyBnsX1DcPD4IQqSDdCRckk7BYnCR4mBSYY13T553sMJP1gQGF8yUFzMs6VE7QYO70gGPVyuzT0o7a62+QMake29Ixx3cXO3pb1lRFcN2FNqFdYAAc1haPqBCk+Ls8nje2VN/+IGyFszgRPNTB0PNha8AcrA= X-Sasl-enc: vMR5prQk/U/0+EXSFmELgxufi/u+Ju8H+cdViBeIYlhz 1294773031 Original-Received: from thinkpad (95-88-32-105-dynip.superkabel.de [95.88.32.105]) by mail.messagingengine.com (Postfix) with ESMTPA id 70C49403521; Tue, 11 Jan 2011 14:10:30 -0500 (EST) In-Reply-To: (joakim@verona.se's message of "Tue, 11 Jan 2011 19:11:48 +0100") User-Agent: Gnus/5.110011 (No Gnus v0.11) Emacs/24.0.50 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:134452 Archived-At: joakim@verona.se 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 @@ img->data.lisp_val)); 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; } else @@ -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; imagemagick_error: 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, init_imagemagick_functions, libraries); } --8<---------------cut here---------------end--------------->8--- Bye, Tassilo