Out of memory in image viewer despite using Image.Dispose

Question

When adding a Next and Previous navigation option to my Image viewer coded in C#, when I press Next about 20 or so times, Visual Studio tells me the process ran out of memory. It does this in any folder with many even if the image file sizes for them all are tiny

I get:

An unhandled exception of type 'System.OutOfMemoryException' occurred in System.Drawing.dll Additional information: Out of memory.

This is the code I am using

private void next_Click(object sender, EventArgs e)
{
    string[] foldernm = Directory.GetFiles(Path.GetDirectoryName(lfoto_file.FileName));
    _pictureIndex++;
    if (_pictureIndex >= foldernm.Length)
    {
        _pictureIndex = 0;
    }
    ibread_img.Image.Dispose();
    ibread_img.Image = Image.FromFile(foldernm[_pictureIndex]);
}

Now as you can see, I have ibread_img.Image.Dispose(); there because I have searched about this and other people said to use that, but it doesn't work and I still get the same problem, a break-point confirms the code is being ran so I am confused to why its still running out of memory. The images I am cycling through are not large. I have tried everything I could find including nulling the previously loaded image, manually calling the garbage collector and nothing seems to work. I am not the best at C# so there might be a horrible mistake or flaw in that code but I don't know, any ideas on how to fix this?

Are the images still being drawn after the exception is thrown? If so, this may be a duplicate of : http://stackoverflow.com/questions/6506089/system-drawing-out-of-memory-exception — JDupont, Aug 29 '16 at 15:42
Since you havent specified a search pattern/filter you are likely trying to load non image files. — Ňɏssa Pøngjǣrdenlarp, Aug 29 '16 at 15:50
The issue was that it doesn't check if the next file is an image or not as LarsTech and Plutonix said, I have made such a silly mistake, I completely forgot and overlooked it, thank you very much — Kai, Aug 29 '16 at 15:52
GDI has a bad habit of throwing `OutOfMemoryException` for all kinds of problems, many of them not related to running out of memory at all. I find it often means the image file is corrupt, or in a format that GDI can't read. — Bradley Uffner, Aug 29 '16 at 18:59

score 1 · Accepted Answer · answered Aug 29 '16 at 18:56

There are a couple of things you could do to improve your viewer. First, you are recreating the list of image files every time; you are loading all of them each time just to access the next one and you dont have to create an image in order to show it.

// class level vars
int picIndex = 0;
IEnumerable<string> files;
int filesCount;
string picPath;
static string[] imgExts = {".png", ".jpg",".gif"};

Since you mentioned a Next and Previous button, you must have almost the same code elsewhere. This will eliminate that duplication, Next:

ShowImage(picIndex);

picIndex+=1;
if (picIndex >= filesCount)
    picIndex = 0;

Then a method to show the desired image:

private void ShowImage(int Index)
{ 
    // create image list if needed (once)
    if (files == null)
    { 
        files = new DirectoryInfo(picPath).EnumerateFiles().
            Where(q => imgExts.Contains(q.Extension.ToLowerInvariant())).
            Select( z => z.FullName);

        filesCount = files.Count();
    }

    string thisFile = files.ElementAt(Index);

    // no need to dispose an image if you never create one          
    pb2.ImageLocation = thisFile;
    lblImgName.Text = Path.GetFileName(thisFile);
}

Rather that create the list of files each time (in 2 places) this does it once ever, and instead of loading a List of all of them, this leaves it as IEnumerable to get them as needed. It also works off FileInfo, is case insensitive mainly to illustrate a different way which would allow you to sort them (OrderBy) by the created date, if you wanted.

Finally, given the full path and file name, you can use the .ImageLocation property and avoid creating and disposing of Images.

The main thing is to minimize the amount of repeated code so you Dont Repeat Yourself. The code for Next and Previous is going to be almost identical.

Thank you, I will try what you said, I do want to optmise and streamline my code as much as possible — Kai, Aug 29 '16 at 21:14

score 0 · Answer 2 · answered Aug 29 '16 at 16:47

Thanks to LarsTech and Plutonix for pointing out my mistake. This new code works fine now:

private void next_Click(object sender, EventArgs e)
    {
        var filteredFiles = Directory.EnumerateFiles(Path.GetDirectoryName(lfoto_file.FileName))
            .Where(file => file.ToLower().EndsWith("jpg") || file.ToLower().EndsWith("png") || file.ToLower().EndsWith("gif") || file.ToLower().EndsWith("bmp") || file.ToLower().EndsWith("tiff") || file.ToLower().EndsWith("ico"))
            .ToList();
        _pictureIndex++;
        if (_pictureIndex >= filteredFiles.Count)
        {
            _pictureIndex = 0;
        }
        ibread_img.Image.Dispose();
        ibread_img.Image = Image.FromFile(filteredFiles[_pictureIndex]);
        init();
    }

I just needed to filter out the correct formats.

Out of memory in image viewer despite using Image.Dispose

2 Answers2