2

I have searched throughout entire Stack Overflow, but I couldn't find an answer to the following:

When I'm using my OpenFileDialog, the files I open get blocked for use out of my program until I close my program. So if I open an image, I am not allowed to replace that image in my Windows Explorer anymore.

I think this is a problem with disposing my OpenFileDialog, but I'm not sure how to solve it...

My code:

using (OpenFileDialog ofd = new OpenFileDialog())
{
    ofd.Title = "Open Image";
    ofd.Filter = "PNG Image(*.png|*.png" +
                 "|GIF Image(*.gif|*.gif" +
                 "|Bitmap Image(*.bmp|*.bmp" +
                 "|JPEG Compressed Image (*.jpg|*.jpg";

    if (ofd.ShowDialog() == DialogResult.OK)
    {
        pictureBox1.Image = new Bitmap(ofd.FileName);
    }
}

I thought that the using block would solve this problem, but nope... It still gets used by the program. I want to load the image in the picturebox and then I want the image to be available again (so I can rename it, replace it, etc...).

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Merlijn
  • 81
  • 2
  • 9

3 Answers3

5

This is not related to the OpenFileDialog. It couldn't possibly be, because the dialog doesn't actually open the file. It just allows the user to select a file to open, and then returns that path to you so that you can write code that opens the file. Besides, you're correctly disposing of the OpenFileDialog through your use of the using statement.

The problem here comes from your actually opening the file—ofd.FileName—as a Bitmap. When you use the Bitmap constructor overload that accepts a path string, the file on disk that contains the image remains locked until the Bitmap object is disposed. So says the documentation:

The file remains locked until the Bitmap is disposed.

Because you're assigning the bitmap to pictureBox1.Image, the Bitmap object will not be disposed until pictureBox1 is disposed. And therefore your image file on disk will remain locked.

If you want to unlock the file, you will either need to make a copy of the bitmap and dispose the original, or clear out the PictureBox and dispose its previous image when you are finished with it.

As I understand your question, it sounds like you want to be able to make changes to the image file on disk while continuing to display the image in the picture box. If that's the case, you will need to make a copy. Do that using the constructor overload that takes an Image, like this:

if (ofd.ShowDialog() == DialogResult.OK)
{
    // Load the image the user selected
    using (Image img = Image.FromFile(ofd.FileName))
    {
        // Create a copy of it
        Bitmap bmpCopy = new Bitmap(img);

        // Clear out the bitmap currently in the picture box,
        // if there is one.
        if (pictureBox1.Image != null)
        {
            pictureBox1.Image.Dispose();
        }

        // Assign the copy of the bitmap to the picture box.
        pictureBox1.Image = bmpCopy;
    }
}
Cody Gray - on strike
  • 239,200
  • 50
  • 490
  • 574
  • 2
    I can confirm this is probably the issue. I've been bitten by it in the past. One option you could consider reading in the file bytes and creating the `Bitmap` from a `MemoryStream` instead. – Chris Sinclair Aug 11 '13 at 13:04
  • @Chris Are you sure that streams don't lock the file? Remember that you have to keep the stream open for the lifetime of any Bitmap object that you create from it. – Cody Gray - on strike Aug 11 '13 at 13:07
  • You got all the votes, you really do need to fix your answer. Bitmap.Clone() creates a shallow copy, it still uses the pixel data of the original bitmap. So doesn't release the lock on the file. Feel free to use my answer and I'll delete mine. – Hans Passant Aug 11 '13 at 13:32
  • @Hans Oops, thanks. I was misremembering that Clone created deep copies of bitmaps. Fixing... – Cody Gray - on strike Aug 11 '13 at 13:33
  • That your code wasn't copy pastable wasn't the problem. I tried this first, making a clone, disposing the old copy and using the clone. But i ended up getting the same error, so i went back to here and checked the second answer, which was xanatos' answer. Thank you very much for taking the effort already, i think i've done something wrong in implementing what you meant!. Thanks a lot really! – Merlijn Aug 11 '13 at 13:34
  • I'll just mention that if I remember right there is a minor problem with all techniques that do not use ImageConverter because otherwise the bit depth is always set to 32 bits and the dpi is set to the dpi of the screen. This normally doesn't matter, but if you want to preserve the bit depth and dpi of the source image it's not so good. But I'm not 100% sure ... – RenniePet Aug 11 '13 at 13:44
  • @CodyGray: No, it won't lock the file. You can read all the file bytes into memory, dispose the original file stream, then create a new `MemoryStream` (or do whatever you want) to the byte array you have in memory. – Chris Sinclair Aug 11 '13 at 22:49
1

As written by Chris, try something like:

pictureBox1.Image = Image.FromStream(new MemoryStream(File.ReadAllBytes(old.FileName)));

It reads all the file with File.ReadAllBytes, put it in a MemoryStream and pass the MemoryStream to the Image static initializer.

Equivalent to:

byte[] bytes = File.ReadAllBytes(old.FileName);
MemoryStream ms = new MemoryStream(bytes);
pictureBox1.Image = Image.FromStream(ms);

You mustn't dispose the MemoryStream! If/when the Image will be disposed, the finalizer of MemoryStream will kick in (if you don't have other references to ms) and the MemoryStream will be disposed (note that this isn't something that will happen immediately... It's something that will happen when the GC will run)

xanatos
  • 109,618
  • 12
  • 197
  • 280
  • Thank you very much. I literally copied your rule of code and it instantly worked, thank you very much!! – Merlijn Aug 11 '13 at 13:23
  • 2
    Oops, guess that's what was wrong my answer: no copy-pastable code. Oh well, happens to me a lot. I still think cloning the Bitmap is a better option than appearing to leak a MemoryStream. That would never get through a code review and needs to be heavily-commented; like you mention, it is quite unintuitive. – Cody Gray - on strike Aug 11 '13 at 13:26
  • @CodyGray I gave you a +1, but the point was that the last mile was given in a comment by Chris (using the `MemoryStream` class). Your response was very "theoretical". – xanatos Aug 11 '13 at 13:30
  • Right, the whole point is that I disagreed with that solution as being optimal. I would have written about it in my answer if I thought it were a good idea. – Cody Gray - on strike Aug 11 '13 at 13:31
  • I'll just mention that if I remember right there is a minor problem with all techniques that do not use ImageConverter because otherwise the bit depth is always set to 32 bits and the dpi is set to the dpi of the screen. This normally doesn't matter, but if you want to preserve the bit depth and dpi of the source image it's not so good. But I'm not 100% sure ... – RenniePet Aug 11 '13 at 13:43
0

The technique I've found to be best is to read the file into a byte array with File.ReadAllBytes() (that opens and closes the file), and then use ImageConverter to convert the byte array into an Image. See here for example: https://stackoverflow.com/a/16576471/253938

Edit: Quote from that previous post of mine: "Some of the other techniques I've tried have been non-optimal because they changed the bit depth of the pixels (24-bit vs. 32-bit) or ignored the image's resolution (dpi)."

Community
  • 1
  • 1
RenniePet
  • 11,420
  • 7
  • 80
  • 106