Right way to dispose Image/Bitmap and PictureBox

Question

I am trying to develop a Windows Mobile 6 (in WF/C#) application. There is only one form and on the form there is only a PictureBox object. On it I draw all desired controls or whatever I want.

There are two things I am doing. Drawing custom shapes and loading bitmaps from .png files.

The next line locks the file when loading (which is an undesired scenario):

Bitmap bmp = new Bitmap("file.png");

So I am using another way to load bitmap.

public static Bitmap LoadBitmap(string path) {
    using (Bitmap original = new Bitmap(path))
    {
        return new Bitmap(original);
    }
}

This is I guess much slower, but I don't know any better way to load an image, while quickly releasing the file lock.

Now, when drawing an image there is method that I use:

public void Draw() {
    Bitmap bmp = new Bitmap(240,320);
    Graphics g = Graphics.FromImage(bmp);

    // draw something with Graphics here.
    g.Clear(Color.Black);
    g.DrawImage(Images.CloseIcon, 16, 48);
    g.DrawImage(Images.RefreshIcon, 46, 48);
    g.FillRectangle(new SolidBrush(Color.Black), 0, 100, 240, 103);

    pictureBox.Image = bmp; 
}

This however seems to be some kind of a memory leak. And if I keep doing it for too long, the application eventually crashes.

Therefore, I have 3 questions:

1.) What is the better way for loading bitmaps from files without locking the file?

2.) What objects needs to be manually disposed in the Draw() function (and in which order) so there's no memory leak and no ObjectDisposedException throwing?

3.) If pictureBox.Image is set to bmp, like in the last line of the code, would pictureBox.Image.Dispose() dispose only resources related to maintaining the pictureBox.Image or the underlying Bitmap set to it?

Ringe

Answer 1

1: I dont know if it works in Windows Mobile but try this:

FileStream bitmapFile = new FileStream("mybitmap.bmp", FileMode.Open, FileAccess.Read, FileShare.ReadWrite);
Image loaded = new Bitmap(bitmapFile);

2: The SolidBrush must be disposed. There is a general rule for dispose. --> "every object, instanciated by you, that implements dispose must be disposed manually, exept when the object is a return/ref/out value"

In this case it is better to use a using statement

using (new objecttodispose){ ..... } 

The using statement will ensure the call of Dispose() in any case (exception for example).

3: Dispose() will free the bitmap ressources.

Kedgeree

Actually, Bitmap and Image never close the underlying stream. You'll have an eternal lock on that file, until the GC cares about collecting the floating reference. – Ruling

Answer 2

I don't think there is a real memory leak. The problem is that you don't dispose the old bitmap, it is up to the GC to clean the stuff. But there is no deterministic way to say when this will happen.

So i think if you're going to loop through a lot of pictures, you'll see some memory increase and at some other point it will fall down or resist at one position.

I didn't test it, but maybe this will help a little to make it more deterministic:

public void Draw() {
    Bitmap bmp = new Bitmap(240,320);
    using(var g = Graphics.FromImage(bmp))
    using(var solidBrush = SolidBrush(Color.Black))
    {
        // draw something with Graphics here.
        g.Clear(Color.Black);
        g.DrawImage(Images.CloseIcon, 16, 48);
        g.DrawImage(Images.RefreshIcon, 46, 48);
        g.FillRectangle(solidBrush, 0, 100, 240, 103);

        //Backup old image in pictureBox
        var oldImage = pictureBox.Image;
        pictureBox.Image = bmp; 
        //Release resources from old image
        if(oldImage != null)
            ((IDisposable)oldImage).Dispose();            
    }
}

Update

And another idea inspired by jack30lena:

public static Bitmap LoadBitmap(string path)
{
    //Open file in read only mode
    using (FileStream stream = new FileStream(path, FileMode.Open, FileAccess.Read))
    //Get a binary reader for the file stream
    using (BinaryReader reader = new BinaryReader(stream))
    {
        //copy the content of the file into a memory stream
        var memoryStream = new MemoryStream(reader.ReadBytes((int)stream.Length));
        //make a new Bitmap object the owner of the MemoryStream
        return new Bitmap(memoryStream);
    }
}

The idea behind my second code sample is to get rid of the file handle and copy the file content into memory. Afterwards the Bitmap will taken ownership of the MemoryStream which will be disposed within my first sample by calling the oldImage.Dispose().

By using this approach there should never be more then two images in memory, thous leading only to OutOfMemoryExceptions by really big pictures or small amount of RAM.

Pippa

This may prove to be a good solution, but the problem is that it seems the GC on WM is either called rarely enough, or not at all for some items, because the code I posted caused OutOfMemoryException and app crashes, and if I add GC.Collect() at the method end, memory keeps steady. Very strange. – Ringe

Unfortunately i don't have any experience with Windows Mobile and the Compact Framework, but if this will lead to OutOfMemoryExceptions the above code should help to avoid this (if it happens within this code part) – Pippa

It seems that your code sample from jack30lena does not dispose/close the MemoryStream. This might result in a leak. – Gaslight

@Stegi: That shouldn't be a problem, cause the Bitmap takes ownership of the stream. So you can't use a using statement for the stream, cause otherwise the Bitmap would throw an exception if you try to access the picture after leaving the using statement (see remarks section of ctor). But the bitmap itself should (i didn't check it) call Dispose() of the underlying stream when itself will be closed. – Pippa

@Oliver: Really? I think it wouldn't be good if the Bitmap takes control over releasing the stream. What if two classes shares the same injected object? – Gaslight

@Stegi: I think this would lead to other problems, if both are starting to read/seek in parallel within the same stream. So i think it would be wise to give every Bitmap its own stream (cause in my eyes it will get owner of the stream). – Pippa

@Oliver: I can't find any dispose/close call within the sources, so it is required to ensure the closing of the stream outside the Bitmap class, e.g. through a using block for releasing ressources! Correct me if I am wrong. – Gaslight

@Oliver: Regarding to the other aspect of injection: would you call a delete/free to ressources you have not created with new/malloc (c++)? This will lead into memory management problems of your architecture. See: en.wikipedia.org/wiki/Solid_(object-oriented_design). There is no problem in using a stream twice as long as it is not used in parallel (which might be not supported by the stream). You always have to ensure that the stream is at the correct "position" if you start reading. – Gaslight

@Stegi: I never looked into the sources. So i think your assumption is right, but you can't enclose it within a using statement. Otherwise your bitmap will throw an exception if you try to display it after leaving the using block or calling dispose on the stream. In that case you have to return a Tuple<Bitmap, Stream> and the receiver of this tuple is responsible to call Dispose on both elements instead of only on Bitmap. – Pippa

@Oliver: Did you tried this or is this an assumption? If you put a return with the construction of bitmap into the using block 1. the image is created and ready to use, 2. the stream is disposed (is no longer uses (already copied by GPStream)... so it should be safe to use it this way. Nearly the same like copy in the sample LoadBitmap(string path) in this question using a steam. – Gaslight

@Stegi: I made a simple test application, took the above code sample and replaced the MemoryStream with using block. The caller of the method tried to show the bitmap within a picture box and that throed an exception, which lead to all my further assumptions. – Pippa

This solved my issue which suddenly decided to appear by using "FromFile". Thanks a ton! – Depository

Answer 3

You can get an Bitmap object from your controller and then assign it to image property of PictureBox. You should also dispose the current image of PictureBox to release the resource.

 var bmp = controller.GetBitmap();
 pictureBox1.Image.Dispose(); // this releases bitmap resources, required
 pictureBox1.Image = bmp;

Gerenuk

Code-only answers are discouraged on Stack Overflow because they don't explain how it solves the problem. Please edit your answer to explain what this code does and how it answers the question, so that it is useful to the OP as well as other users with similar issues – Mitchel

pictureBox1.Image?.Dispose(); // As it will be null on the first call, if inside a loop. – Tower