Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Huge memory consumption with latest imagemagick@6 update #1401

Closed
ccbcreg opened this issue Jul 7, 2023 · 27 comments · Fixed by #1406
Closed

Huge memory consumption with latest imagemagick@6 update #1401

ccbcreg opened this issue Jul 7, 2023 · 27 comments · Fixed by #1406

Comments

@ccbcreg
Copy link

ccbcreg commented Jul 7, 2023

Description

Long running ruby process creating images with rmagick consumes massive amounts of memory since most recent ImageMagick-6.9.12-90.tar.gz release.

Steps to Reproduce

Install latest imagemagick@6 (https://formulae.brew.sh/api/formula/imagemagick@6.json) and install rmagick 5.2.0. Long running ruby process to create thousands of graph images. Process consumes way more memory that cripples host machine.

I'm suggesting this memory issue is a result of the latest security updates to ImageMagick 6. I understand this might not be the right place for this issue but I figured I'd start here first.

/usr/local/Cellar/imagemagick@6/6.9.12-67 => 248mbs of memory that plateaus out.
/usr/local/Cellar/imagemagick@6/6.9.12-90 => 7Gigs of memory that keeps accruing exponentially

This issue has come up since the latest ImageMagick release (ImageMagick-6.9.12-90.tar.gz) which came out June 25th 2023 and subsequent Ubuntu packages (imagemagickwand-dev [8:6.9.10.23+dfsg-2.1ubuntu11.9]) and MacOS (imagemagick@6 /usr/local/Cellar/imagemagick@6/6.9.12-90).

System Configuration

Ubuntu 20 LTS (8:6.9.10.23+dfsg-2.1ubuntu11.9 [security]: amd64 i386)
MacOS 12.6.7 (https://formulae.brew.sh/api/formula/imagemagick@6.json)

  • ImageMagick version: ImageMagick-6.9.12-90
  • RMagick version: 5.2.0
  • Ruby version: 3.1.2
  • Environment (Operating system, version and so on): MacOS 12.6.7, Ubuntu 20 LTS
  • Additional information:

In both MacOS and Ubuntu 20 LTS with the same code ruby app memory consumption increases exponentially until, in the case of Ubuntu 20 LTS, the process is killed.

https://www.ubuntuupdates.org/package/core/focal/universe/updates/imagemagick

imagemagickwand-dev [8:6.9.10.23+dfsg-2.1ubuntu11.7] works great!
imagemagickwand-dev [8:6.9.10.23+dfsg-2.1ubuntu11.9] memory hog, kills process

https://github.com/Homebrew/homebrew-core/blob/master/Formula/imagemagick@6.rb

[imagemagick@6: update 6.9.12-67 bottle.]https://github.com/Homebrew/homebrew-core/blob/b8ea6da2121433feb4fb67a47b2600c9e54e7402/Formula/imagemagick%406.rb works great!

imagemagick@6: update 6.9.12-90 bottle. memory hog, kills process

Steps I've tried to free up memory

Tried to use #destroy!

  image = Magick::Image.read(file)
  image.format = "PNG"
  image.write("/home/png/#{File.basename(file, '.*')}.png")
  image.destroy!

Tried to nil out image object

image = Magick::Image.read(file)
image.format = "PNG"
image.write("/home/png/#{File.basename(file, '.*')}.png")
image = nil

and even GC.start

image = Magick::Image.read(file)
image.format = "PNG"
image.write("/home/png/#{File.basename(file, '.*')}.png")
GC.start

Memory consumption increases exponentially with newest imagemagick security release.

@mockdeep
Copy link
Member

mockdeep commented Jul 7, 2023

This looks like maybe an issue that should be reported over on the ImageMagick repo. @Watson1978 @dlemstra thoughts?

@dlemstra
Copy link
Member

dlemstra commented Jul 7, 2023

This sounds like a memory leak and it is probably happening on the ImageMagick side. But please first figure out where the leak is happening before creating an issue. It could also be an issue on the RMagick side.

@Watson1978
Copy link
Member

@ccbcreg Hmm, I can't reproduce your problem with your code. I need more detailed information you handled about format of the file, etc.

@ccbcreg
Copy link
Author

ccbcreg commented Jul 12, 2023

Hi there. Thank you for getting back to me, I appreciate your efforts and time. Re: specifics of my app that uses RMagick (rmagick 5.1.0), it's a Rails app (7.0.4) with a rake task that creates graphs PNGs. The code has worked with many versions of Ruby, 2 thru 3.1.2, with many versions of Rails and equally many versions of ImageMagick for 10+ years. There is a rake task that produces several thousand images and has never taken that much memory in the past. It just worked until Imagemagick@6/6.9.12-90.

I've been able to downgrade the Homebrew formula to ImageMagick-6.9.12-89 (https://github.com/Homebrew/homebrew-core/blob/a11ef4c74f71eda747b9cbd17205c9c7ac9f1040/Formula/imagemagick%406.rb) and everything works fine. I went back to ImageMagick-6.9.12-67 and everything worked fine. I never changed the ruby code after trying what I mentioned in my original post above. I was also able to downgrade the Ubuntu 20 LTS package as well which works too.

ccbcreg@hostyhost:~$ apt policy libmagickwand-dev
libmagickwand-dev:
  Installed: 8:6.9.10.23+dfsg-2.1ubuntu11.9
  Candidate: 8:6.9.10.23+dfsg-2.1ubuntu11.9
  Version table:
 *** 8:6.9.10.23+dfsg-2.1ubuntu11.9 500
        500 http://mirror.rackspace.com/ubuntu focal-updates/universe amd64 Packages
        500 http://mirror.rackspace.com/ubuntu focal-security/universe amd64 Packages
        100 /var/lib/dpkg/status
     8:6.9.10.23+dfsg-2.1ubuntu11 500
        500 http://mirror.rackspace.com/ubuntu focal/universe amd64 Packages

Then downgrade to the 8:6.9.10.23+dfsg-2.1ubuntu11 version via

sudo apt-get install imagemagick=8:6.9.10.23+dfsg-2.1ubuntu11 -y --allow-downgrades 
sudo apt-get install imagemagick-6-common=8:6.9.10.23+dfsg-2.1ubuntu11 -y --allow-downgrades 
sudo apt-get install libmagickcore-6-headers=8:6.9.10.23+dfsg-2.1ubuntu11 -y --allow-downgrades 
sudo apt-get install libmagickcore-6-arch-config=8:6.9.10.23+dfsg-2.1ubuntu11 -y --allow-downgrades 
sudo apt-get install libmagickcore-6.q16-6=8:6.9.10.23+dfsg-2.1ubuntu11 -y --allow-downgrades 
sudo apt-get install libmagickcore-6.q16-6-extra=8:6.9.10.23+dfsg-2.1ubuntu11 -y --allow-downgrades 
sudo apt-get install libmagickcore-6.q16-dev=8:6.9.10.23+dfsg-2.1ubuntu11 -y --allow-downgrades 
sudo apt-get install libmagickwand-6-headers=8:6.9.10.23+dfsg-2.1ubuntu11 -y --allow-downgrades 
sudo apt-get install libmagickwand-6.q16-6=8:6.9.10.23+dfsg-2.1ubuntu11 -y --allow-downgrades 
sudo apt-get install libmagickwand-6.q16-dev=8:6.9.10.23+dfsg-2.1ubuntu11 -y --allow-downgrades 
sudo apt-get install libmagickwand-dev=8:6.9.10.23+dfsg-2.1ubuntu11 -y --allow-downgrades 

I'm not proud of my capitulation but I had to get back on track and produce builds. @mockdeep and @dlemstra, I tend to agree that this is a ImageMagick repo thing but I haven't a clue how to debug C much less track down the memory leak between those two releases, ImageMagick-6.9.12-89 to ImageMagick-6.9.12-90. I'm out of my depth there. However, I can say with a bit of confidence that I don't think it is something on the rmagick/ruby side since the negative behavior is only present with one version of ImageMagick. I thought at the very least I should point it out and bring it to your attention. If I have time, I'll create an example and see if I can demonstrate the issue with real code.

Thank you again for your time, much appreciated.

@dlemstra
Copy link
Member

It would help if you could constrain how this can be reproduced. Is it a specific input image that reproduces this? Does this only happen when the output format is png?

@ccbcreg
Copy link
Author

ccbcreg commented Jul 12, 2023

No input image, I'm using the gruff gem to produce graphs populated with data from a database. All of the images are png format, I've never used any other image format. I'll see if I can produce a rails app that can reproduce the problem.

@Watson1978
Copy link
Member

@ccbcreg Thank you for the info. I will try again at this weekend. ( If you can create the Dockerfile, it will help us to reproduce the issue. )

@ccbcreg
Copy link
Author

ccbcreg commented Jul 12, 2023

Okay, I've produced a simple Rails app using the same gruff gem and iterates over the example from the gruff README.md. When run, you should see the memory usage of that process blow up until the process is killed.

When I downgrade the ImageMagick version to less than ImageMagick-6.9.12-90. Everything works peachy. I'm sorry I don't have a Dockerfile for you @Watson1978. I don't know how to create a Docker image quickly enough to produce this example.

Please let me know what else I can do or if you have any questions re: the example.

@Watson1978, perhaps worth noting. I've tried this app with both gruff version 0.8.0, which was the version I'm currently using in instance, and gruff version 0.22.0. Both fail with ImageMagick-6.9.12-90 and both work with less than ImageMagick-6.9.12-90.

@Watson1978
Copy link
Member

Watson1978 commented Jul 14, 2023

@ccbcreg Thanks for your works!

Seems certainly memory usage increases linearly when we use Magick::Draw to render image.

Sample

require 'rmagick'

3000.times do |i|
  img = Magick::Image.new(1000, 1000)

  gc = Magick::Draw.new
  gc.text(0, 67, 'Hello world!!')
  gc.draw(img)
end

Result

@ccbcreg
Copy link
Author

ccbcreg commented Jul 14, 2023

Yep, that's what I'm seeing too. Thank you @Watson1978 for taking a look.

@Watson1978
Copy link
Member

At least, this memory leaks has occurred since ImageMagick-6.9.12-78 on my environment.

If I revert magick/draw.c changing at ImageMagick-6.9.12-78, the memory leaks were stopped.
ImageMagick/ImageMagick6@6.9.12-77...6.9.12-78

@dlemstra Can you look magick/draw.c changing ? I don't understand details of changing yet

@dlemstra
Copy link
Member

Thanks for all the information, will take a look at this today. Were you also able to reproduce this on the command line @Watson1978?

@Watson1978
Copy link
Member

Looks for me that draw_info->image_info=CloneImageInfo(image_info); line is one of memory leaks, at least.
スクリーンショット 2023-07-15 16 13 51

@Watson1978
Copy link
Member

Thanks for all the information, will take a look at this today. Were you also able to reproduce this on the command line @Watson1978?

  1. save the sample code in Huge memory consumption with latest imagemagick@6 update #1401 (comment) as rmagick.rb
  2. run ruby rmagick.rb on your terminal

@Watson1978
Copy link
Member

diff --git a/magick/draw.c b/magick/draw.c
index cc62082d2..2eea18deb 100644
--- a/magick/draw.c
+++ b/magick/draw.c
@@ -382,6 +382,8 @@ MagickExport DrawInfo *CloneDrawInfo(const ImageInfo *image_info,
     clone_info->composite_mask=CloneImage(draw_info->composite_mask,0,0,
       MagickTrue,&draw_info->composite_mask->exception);
   clone_info->render=draw_info->render;
+  if (clone_info->image_info != (ImageInfo *) NULL)
+    clone_info->image_info=DestroyImageInfo(clone_info->image_info);
   clone_info->image_info=CloneImageInfo(draw_info->image_info);
   clone_info->debug=draw_info->debug;
   return(clone_info);

At least, looks for me that CloneDrawInfo() requires freeing because clone_info->image_info has allocated memory by draw_info->image_info=CloneImageInfo(image_info);.

Better, but the memory leak is still not completely gone....

@dlemstra
Copy link
Member

I can reproduce this issue inside a devcontainer that has memory leak detection with the following command convert -size 100x100 xc:red -draw "text 0,0 'Test'" info:. I am getting this output:

==23898==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 16792 byte(s) in 1 object(s) allocated from:
    #0 0x55abf1cca31e in malloc (/ImageMagick6/bin/convert+0x40d31e) (BuildId: f3ec29cd8d94404a9ac8abc138264e7e97337fb7)
    #1 0x55abf1d2f3a5 in AcquireMagickMemory (/ImageMagick6/bin/convert+0x4723a5) (BuildId: f3ec29cd8d94404a9ac8abc138264e7e97337fb7)
    #2 0x55abf1d0fe30 in AcquireImageInfo (/ImageMagick6/bin/convert+0x452e30) (BuildId: f3ec29cd8d94404a9ac8abc138264e7e97337fb7)
    #3 0x55abf1d15b08 in CloneImageInfo (/ImageMagick6/bin/convert+0x458b08) (BuildId: f3ec29cd8d94404a9ac8abc138264e7e97337fb7)
    #4 0x55abf20a1c68 in GetDrawInfo (/ImageMagick6/bin/convert+0x7e4c68) (BuildId: f3ec29cd8d94404a9ac8abc138264e7e97337fb7)
    #5 0x55abf20a28a1 in CloneDrawInfo (/ImageMagick6/bin/convert+0x7e58a1) (BuildId: f3ec29cd8d94404a9ac8abc138264e7e97337fb7)
    #6 0x55abf20af265 in RenderMVGContent draw.c

I suspect that we don't destroy the CloneDrawInfo. I will investigate this and come back to you.

@dlemstra
Copy link
Member

This commit should resolve the issue: ImageMagick/ImageMagick6@c90e79b. It fixed the issue on the command line. Does this not resolve your issue @Watson1978?

@Watson1978
Copy link
Member

Watson1978 commented Jul 16, 2023

I tried HEAD of main in ImageMagick 6, however, seems it still has the memory leak little by little.

@Watson1978
Copy link
Member

Watson1978 commented Jul 16, 2023

Now, RMagick calls AcquireDrawInfo() and GetDrawInfo().

rmagick/ext/RMagick/rmdraw.c

Lines 1412 to 1431 in e7d5148

DrawOptions_initialize(VALUE self)
{
Draw *draw_options;
TypedData_Get_Struct(self, Draw, &rm_draw_data_type, draw_options);
draw_options->info = AcquireDrawInfo();
if (!draw_options->info)
{
rb_raise(rb_eNoMemError, "not enough memory to continue");
}
GetDrawInfo(NULL, draw_options->info);
if (rb_block_given_p())
{
rb_yield(self);
}
return self;
}

Seems GetDrawInfo() will cause memory leak...

MEMO)

AcquireDrawInfo() has called GetDrawInfo() internally.

MagickExport DrawInfo *AcquireDrawInfo(void)
{
  DrawInfo
    *draw_info;

  draw_info=(DrawInfo *) AcquireCriticalMemory(sizeof(*draw_info));
  GetDrawInfo((ImageInfo *) NULL,draw_info);
  return(draw_info);
}

https://github.com/ImageMagick/ImageMagick6/blob/24a88a922df011830dde8329825b6ded73209db8/magick/draw.c#L235C1-L243C2

So, RMagick's GetDrawInfo() calling is unnecessary, maybe...

GetDrawInfo(NULL, draw_options->info);

@Watson1978
Copy link
Member

Looks for me that GetDrawInfo() is not considered to be called multiple times.

  (void) memset(draw_info,0,sizeof(*draw_info));
  draw_info->image_info=CloneImageInfo(image_info);

draw_info->image_info might already have allocated memory. However, it reset an address by memset() and allocate new memory....

@Watson1978
Copy link
Member

I think #1406 will fix memory leak. @dlemstra Can you review the PR?

@bastien-roucaries
Copy link

Asked for a CVE. Will give you the number ASAP

@bastien-roucaries
Copy link

This commit should resolve the issue: ImageMagick/ImageMagick6@c90e79b. It fixed the issue on the command line. Does this not resolve your issue @Watson1978?

This commit fix CVE-2023-39978.

@bastien-roucaries
Copy link

Still waiting for rmagick CVE

@bastien-roucaries
Copy link

rmagick one is CVE-2023-5349

@janklimo
Copy link

janklimo commented Nov 5, 2023

I tried HEAD of main in ImageMagick 6, however, seems it still has the memory leak little by little.

@Watson1978 would you mind sharing what tool you're using for this? Slick!

@Watson1978
Copy link
Member

@janklimo It is Instruments bundled in Xcode

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants