U4-6246 - Uploading specific images causes GDI+ Errors

Created by Helmuth Bederna 09 Feb 2015, 13:36:55 Updated by Shannon Deminick 18 May 2015, 01:46:35

Relates to: U4-4049

Hello Guys,

at them moment I'm working on a Package called Media Upload with Progressbar. While working on this I noticed an issue with the function MediaService.Save(...) throwing the generic GDI+ exception "Out of memory" So I decided to test those images via the Core Vanilla uploader to check what I'm doing wrong. Unfortunately it seems that this uploader has the same problem.

So I downloaded the GitHub Head for version 7.2.2, build the Project and copied the dll's + symbol files into my test installation. Then I uploaded the Images and found that the following line is throwing the error. (See Screenshots)

There are also a lot more information about this case in the ticket: http://issues.umbraco.org/issue/U4-4049

My Environment: Windows 7 x64 Umbraco 7.2.2 IIS Express

The Images will follow asap!

Please let me know if u need more information!

Best regards, Helmuth

7 Attachments

Download lightair_uploads.zip

Comments

Helmuth Bederna 09 Feb 2015, 15:49:22

I just did some more tests to help you find the issue. At the moment it seems like it only happens on IIS Express for me. The reason is the high memory usage when re sizing the images or to be precise the GenerateThumbnail(..) method.

The simple umbraco installation I have, usually runs with around 150 - 170 MB RAM but as soon as I upload an image it starts using around 350 - 450 MB. So when this memory isn't freed in time and there is another image processed the memory exceeds the limits of the IIS Express.

I've tried to add a GC.Collect() at the beginning and the end of the method which seems to improve it a little bit but still causes the IIS Worker to crash.

I've also tried to start the IISExpress as a x64 app but I didn't get it working.

This issue might also occur on normal IIS instances when a large enough image is uploaded causing a really high memory usage

That's all I found out so far. Maybe someone has an idea to get rid of the high memory usage while uploading an image.


Bjarne Fyrstenborg 09 Feb 2015, 18:32:07

I can confirm this issue, so I have attached a couple of zip-files with a bunch of images, where it has failed to upload all images because of "Out of memory." and "GDI+ error".


Bjarne Fyrstenborg 09 Feb 2015, 18:38:36

Some other images: http://dev.bjarnefyrstenborg.dk/lightair_in_details.zip http://dev.bjarnefyrstenborg.dk/lightair_ionflow.zip


Shannon Deminick 10 Feb 2015, 04:34:51

Will have a look, almost positive this is this same issue: http://issues.umbraco.org/issue/U4-3844

which was fixed before but was done so based on a 'guess' because there's no way to really know the settings that should be used to keep the best quality with certain images sizes. The Out of memory exception is almost certainly not 'out of memory' but that is the underlying exception that GDI+ decides to throw. I'll investigate but it would be worth reading through that other issue I mentioned for background info.


Shannon Deminick 10 Feb 2015, 05:38:04

So this issue might actually turn out to be a real out of mem exception! :) GDI+ is terrible when it comes to exceptions and seems to throw that for all exceptions. In rev: 0efb9b72e75c8a14a6dc6989d746e4cfbe6f84a4 I've added a small EXIF reader so that we can attempt to read the width/height for JPG files based on it's meta data. This means that we don't have to load the image into memory using GDI just to get width/height.

However, this 'fix' is based on the stack trace that doug provided on the other issue but other stack traces show that this same exception occurs with ImageProcessor. In which case I'm not actually sure there's anything that can be done with such large files since we have no control over GDI. I haven't been able to replicate this issue today yet so will investigate with web matrix and see what happens. Will let you know.


Helmuth Bederna 10 Feb 2015, 05:58:36

Hi! Yeah the GDI+ is pretty bad at throwing good exceptions. :D Well anyway it's definitely a good idea to get the image info out of the meta data. Another optimization would be to not run the resize / thumbnail method to save them and not really resize them. Eg. an image with a size of 6000x6000 is resized to 6000x6000 :) Also try to open some Visual Studio instances and fill up your RAM till you almost reach the limits (~1.5Gb remaining). Then start the tests with the IIS Erpress. It needs to be an IIS Express because the normal IIS works much better and the OOM are pretty rare!

If I can help you with anything please let me know!


Shannon Deminick 10 Feb 2015, 06:45:04

I'm not sure what you mean about this?

Eg. an image with a size of 6000x6000 is resized to 6000x6000

We do create 2 real thumbnails, one at 100px and one at 500px, these thumbnails are sort of legacy now due to the use of ImageResizer to generate the thumbnails but are still widely used so we cannot get rid of these at the moment.


Helmuth Bederna 10 Feb 2015, 06:57:36

Hm... I'll need to test once more but last time I debugged it seems the method GetThumbnail... is called 3 times. And the 1st time with the info it should keep the same size. I might be wrong. But it looked like.


Helmuth Bederna 10 Feb 2015, 07:18:28

Sry. my mistake. Forget what I said :)


Helmuth Bederna 10 Feb 2015, 07:44:14

Shannon,

I just tried something else and I think we are probably looking at the wrong part of the source code. Because the resize causes a memory peak around 50 MB which is a lot but not really bad.

The really funny stuff starts right after the SetFileOnContent(...) method. Because over here it steps into the Method Save(...) in the MediaService.cs. As soon the Line '''932''' (Saving.IsRaisedEventCancelled(...)) is called the memory goes from ~190 MB up to 450 and more. I can't debug any further because it's probably anywhere else or I just can't follow the event. Would be cool if you can give me a hint whats happening in the attached events. ;)


Shannon Deminick 10 Feb 2015, 07:48:14

In that method it does

 using (var originalImage = Image.FromStream(fileStream))

which loads the image into memory, it does so in order to create those two thumbnails and while it's open it also collects the image sizes. Before the exif stuff mentioned above, in the same request, two calls would be made to:

 using (var originalImage = Image.FromStream(fileStream))

at different times so loading and unloading and loading again into mem. Now it only loads once which i understand is still an issue but the reality is that if we are ever to make thumbnails of any image, the above code needs to execute... this is definitely the line of code that causes the mem increase.


Helmuth Bederna 10 Feb 2015, 08:31:30

Well it's not my call stack. I've attached a few images to show you how it looks like over here. Step1 - Step3 is the call stack. I've made the screenshots in the order they And the memory always seems to be ok. Like I said before only a peak around 50 MB or so. The last image (Step4) is after all the stuff is done. The using line you referring on isn't called anymore. My problem is what's happening after this. Are there some other events called. Probably they are no longer within the umbraco core solution so I can't step with it.


Shannon Deminick 13 Feb 2015, 05:25:56

Right, so you mem increase of ~50mb is normal when opening the file to save the 2 thumbnails, that's approx what I get but depends on the file size. The line i mentioned above will be executing... it is just below the line referenced in your step1 (and the stack inside of your step2 is inside of that using block). But that's ok, 50mb is fine and there's nothing we can do about it really.

However, if your memory is going up inside of MediaService.Save and is based on some event handler... unfortunately it means you have some detective work to do to see what is causing that. Until I added the EXIF 'fix', it was indeed re-loading the image into memory to get it's size, see these handlers:

https://github.com/umbraco/Umbraco-CMS/blob/dev-v7/src/Umbraco.Web/PropertyEditors/ImageCropperPropertyEditor.cs#L201

https://github.com/umbraco/Umbraco-CMS/blob/5b9a98ad6ae9e63322c26f7b162204e34f7fcb54/src/Umbraco.Web/PropertyEditors/FileUploadPropertyEditor.cs#L157

You can break point in either of these to see what is happening. If your image is not a jpg it wil be re-loaded into memory to get the file size (since exif data is not available). Now i know you are wondering why the image size is being checked twice - the reason is because we need to open the image to save the thumbnails, so might as well get the image size while were doing that. However, we also need to get the image size based on events because developers might save media without using the SetFileOnContent method.

If you can let me know if you find anything with those event handlers, it will be helpful, I need to close this issue so we can release 7.2.2


Helmuth Bederna 13 Feb 2015, 07:30:05

Hi Shannon,

I totally agree with you in everything. :) The ~50 MB of course depends on how large the images are and I don't think the issue is located within the Umbraco Core Media source files. I also understand why you guys are opening the files multiple times and as long the image class is disposed at the end of it's usage, like you do, there shouldn't be any memory related issue. If the memory is needed it is freed by the GC anyway.

Anyway, while reading you comment I thought about what are the main differences between my two installations. Well, the main difference between my two test systems is that one uses an SQL Express Server and the other an SQL CE Database. So I thought back to my desktop app developer days where I had a lot of memory troubles with the SQL CE. So maybe the issue is anywhere in there. But it is just a guess and I might be completely wrong! Probably it's worth it to do a memory profiler session on day. :)

So yeah, I'm completely fine with closing this ticket as the problem only occurs on IIS Express installations with an SQL CE database which shouldn't be used in live scenarios.

If I do have some more information or a memory dump you can look at. I'll open up a new ticket for further investigation.

Thanks for spending your time on trying to fix it!

Best regards and a wonderful weekend, Helmuth


Andrew Sanin 15 May 2015, 12:14:59

hi @Shandem It seems that because of your fix with reading Exif image dimensions prior to image file dimensions I'm having problems on my project. I'm uploading an image resized from original. Exif metadata contains original image dimensions, but File has much smaller actual image dimensions: http://screencast.com/t/KdoSoWQCVOB

There should be a way how to disable your fix, or configure it. Would appreciate any help.

/Andrew Sanin


Shannon Deminick 18 May 2015, 01:46:35

@andrewsanin can you resize the image with software that updates the Exif properly?


Priority: Normal

Type: Bug

State: Closed

Assignee: Shannon Deminick

Difficulty: Very Difficult

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions: 7.2.1

Due in version:

Sprint:

Story Points:

Cycle: