6

I have an application, which processes and re-sizes images and occassionally during long iterations I get OutOfMemoryException.

I store my images in the database as filestream and during processing I need to save them to a temporary physical location.

My models:

[Table("Car")]
public class Car
{
   [... some fields ...]
   public virtual ICollection<CarPhoto> CarPhotos { get; set; }
}

[Table("CarPhoto")]
public class CarPhoto
{
   [... some fields ...]
   public Guid Key { get; set; }

   [Column(TypeName = "image")]
   public byte[] Binary { get; set; }
}

Processing looks roughly like this:

foreach (var car in cars)
{
    foreach (var photo in car.CarPhotos)
    {
        using (var memoryStream = new MemoryStream(photo.Binary))
        {
            using (var image = Image.FromStream(memoryStream)) // this is where the exception is thrown
            {
                var ratioX = 600.00 / image.Width;

                var newWidth = (int)(image.Width * ratioX);
                var newHeight = (int)(image.Height * ratioX);

                using (var bitmap = new Bitmap(newWidth, newHeight))
                {
                    Graphics.FromImage(bitmap).DrawImage(image, 0, 0, newWidth, newHeight);
                    bitmap.Save(directory + filePath);
                }
            }
        }
    }
}

I've looked at this similar thread, but none of the answers seems to apply in my case.

One of the answers suggests using Image.FromStream(), but it's what I'm doing anyway.

I'm quite confident all my images are valid. The exception seems to be occurring randomly, most often when it comes to processing larger files, but it happens for smaller ones as well. Sometimes one image will fail, but it will be processed fine the next time.

As far as I can see, I'm disposing of everything correctly (memory stream, image and bitmap).

The job is being trigerred by Hangfire. Could that possibly cause a problem?

Community
  • 1
  • 1
Jerry
  • 1,762
  • 5
  • 28
  • 42
  • I think you need to make yourself familiar with windbg and analyse crash dumps to find a solution. – weismat Jul 10 '15 at 08:57
  • Don't load all the images in memory before processing. The advantage of FILESTREAM storage is that you get back a stream instead of a BLOB. Use the SqlFileStream to load and process the images one at a time. – Panagiotis Kanavos Jul 10 '15 at 09:13
  • That Graphics object needs to be disposed as well, use *using* like you did with the other objects. Non-zero odds that this doesn't fix the problem and your program dies due to address space fragmentation, bitmaps are difficult objects because they are usually so large. Running this code in 64-bit mode is highly recommended. If that is not an option then re-using the MemoryStream object instead of re-creating it repeatedly would be wise. – Hans Passant Jul 10 '15 at 09:15

3 Answers3

6

Your program eats a lot of memory, getting OOM is certainly not unexpected. Exactly where it will die is unpredictable. But yes, creating the bitmap is where it is likely to die first. Addressing the most likely reasons in order:

   foreach (var car in cars)

There is no obvious upper-bound on the number of cars you handle. Each car has multiple images and you appear to store those images in memory (photos.Binary). Or in other words, this program is guaranteed to die sooner or later, simply because it needs to handle an ever-more increasing number of cars. Only way to get ahead is by processing cars serially instead of batching them all in memory. Likely to be unpleasant advice, running this code in 64-bit mode is highly recommended.

   using (var memoryStream = new MemoryStream(photo.Binary))

That memory stream is a big problem, its underlying buffer is very likely to be stored in the Large Object Heap. The LOH is not compacted, re-allocating the MemoryStream repeatedly makes it likely that this heap is getting fragmented. Sooner or later you run out of a hole that's big enough to fit the buffer for the next photo. Exceedingly random, exactly when that happens depends on what kind of photos you processed earlier. You'll want to re-use the object instead of re-allocating it over and over again. Just create it once before you enter the loop, set the Capacity to a nice big number.

    Graphics.FromImage(bitmap).DrawImage(image, 0, 0, newWidth, newHeight);

That Graphics object needs to be disposed. Use the using statement like you did with the other objects.


By and large, the real problem with your program is that it is simply doesn't scale and will always fall over when it needs to deal with an increasing dataset. Fixing that can be a very significant re-write, you almost surely want to flip the ignore bit on that and take advantage of the available address space in a 64-bit process. Hardware to the rescue, it is readily available today.

Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536
  • Thanks for a comprehensive answer. I like the idea of re-using the memory stream. Never thought re-allocating it could be such a big issue. – Jerry Jul 10 '15 at 10:21
0

You can avoid doing all the Bitmap code (which may solve the memory issue) by just using:

var resized = image.GetThumbnailImage(newWidth, newHeight,() => false, IntPtr.Zero);
resized.Save(directory + filePath);
Kevin
  • 2,281
  • 1
  • 14
  • 16
  • 1
    I have tried it - makes sense not to create Bitmap and Graphics objects unnecessarily, but I've got the same exception in the end. Seems that I need to follow the advice from Hans's answer. – Jerry Jul 10 '15 at 10:04
0

I had the same problem.The problem is that Image Object does not dispose immediately even if you use USING statement.For example

 IEnumerable<Control> mcontrols = this.panel1.Controls.Cast<Control>().Where(n => n.GetType() == typeof(PreviewImage));
        // for unclear reason after the dispose needs many times to clear everything;
        while (mcontrols.Count() != 0)
        {
            foreach (PreviewImage pi in mcontrols)
            {
                pi.Dispose();
            }
            mcontrols = this.panel1.Controls.Cast<Control>().Where(n => n.GetType() == typeof(PreviewImage));
            if (mcontrols == null) {
                break;
            }

        }

even after first loop still in my panel1 I had not desposed controls.Yes it is horrendous workaround but it works for me