5

I've been trying to sort out some of our code using Dispose properly in a number of places that things were being left hanging around. Once such instance was Icons, and something I noticed which I thought was odd, if I call Icon.Dispose() I was still able to use the Icon.

So I extracted it out into a little console application that I fully expected to crash (throw an ObjectDisposedException), but it didn't... Am I mis-understanding what dispose should be doing here?

using System;
using System.Collections.Generic;
using System.Text;
using System.Drawing;
using System.IO;

namespace DisposeTest
{
    class Program
    {
        static void Main(string[] args)
        {
            Icon icon = new Icon(@"C:\temp\test.ico");
            icon.ToBitmap().Save(@"C:\temp\1.bmp");
            icon.Save(new FileStream(@"C:\temp\1.ico", FileMode.OpenOrCreate, FileAccess.ReadWrite));
            icon.Dispose();
            GC.Collect(); // Probably not needed, but just checking.
            icon.Save(new FileStream(@"C:\temp\2.ico", FileMode.OpenOrCreate, FileAccess.ReadWrite));
            icon.ToBitmap().Save(@"C:\temp\2.bmp");
        }
    }
}
Ian
  • 33,605
  • 26
  • 118
  • 198
  • I think Icon is not collected here because there is still reference pointing to it... – Darius Kucinskas Aug 16 '11 at 13:08
  • 1
    Seems that Save() and ToBitmap() use a byte[] called iconData internally, which isn't touched in Dispose(). Not quite sure the reasoning behind it all though. – Ian Aug 16 '11 at 13:08
  • 1
    @Darius: This isn't about garbage collection. It's about disposal. The `GC.Collect()` is a totally unrelated issue. – recursive Aug 16 '11 at 13:09
  • @recursive Dose that mean that release of unmanaged resource happens then Dispose() is called? – Darius Kucinskas Aug 16 '11 at 13:15
  • @Darius: To the best of my knowledge, it should. But that's not enforced by the language. It's just enforced by convention and the implementation of `Icon`'s `Dispose` method. – recursive Aug 16 '11 at 13:54

4 Answers4

8

I extracted it out into a little console application that I fully expected to crash, but it didn't. Am I mis-understanding what dispose should be doing here?

Programs that contain bad programming practices are not required to crash and burn. It sure is nice if they do, but they do not have to. To my knowledge the documentation does not state that "program crashes if you use a disposed icon" is a supported feature of the library, so you cannot rely on it.

(If the documentation does say that somewhere then you've found a bug; if you feel like reporting it on Connect, that would be appreciated.)

For a related question that people seem to find entertaining, see this question about taking advantage of non-crashing behaviour in C:

Can a local variable's memory be accessed outside its scope?

Finally, your comment is correct in noting that the garbage collector is probably not relevant. Remember, the point of "Dispose" is to throw away unmanaged resources. The point of the garbage collector is to throw away managed memory and to throw away unmanaged resources via finalizers. Since the icon's memory is still alive (and the disposer will probably have suppressed finalization regardless), forcing a garbage collection here is unlikely to do anything.

Community
  • 1
  • 1
Eric Lippert
  • 647,829
  • 179
  • 1,238
  • 2,067
  • Thanks for the response, crash was a bad word to use. In essence, I was expecting an ObjectDisposedException and was very surprised not to receive one, how that is then handled I suppose varies. Your point about throwing away unmanaged resources I think is where the key is at. Seems in the case of using a file path to construct the icon the data is thrown into a managed byte[], and can continue working without icon handles etc. in this case. – Ian Aug 16 '11 at 13:59
7

It doesn't crash because the Dispose() method doesn't do anything in your case. The Icon class wrapper can wrap two distinct sources of icon data. In your case, the icon data comes from a file and is stored in a byte[]. Which is not a system resource that requires disposing, the garbage collector already takes care of it.

The other icon data source is from a Windows handle. Fairly rare, but you'll get that when using Icon.FromHandle() or one of the System.Drawing.SystemIcons. Now it does wrap an unmanaged resource that makes using Dispose() useful.

This pattern is not unusual. There are also plenty of cases where Dispose() is inherited into a class where it doesn't make much sense. Like MemoryStream or BackgroundWorker. The conventional wisdom is to dispose them anyway.

Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536
  • Hans, a MemoryStream will throw if you try anything after a Dispose(). The Backbroundworker does indeed not. – H H Aug 16 '11 at 16:07
4

This behaviour, while a little strange-looking, does make some sense. Usually, Dispose() is used to release unmanaged resources; it's a way to "play nice" with other processes in the system. In this case, you're likely releasing the HICON handle during Dispose(). The rest of the (managed) resources associated with the Icon (including the actual byte[] data) will be release when the managed object is collected.

You try to do that by calling GC.Collect(), but that's not going to do anything, since the Icon is still alive: it's referenced by the local variable called icon, and is not currently eligible for collection.

dlev
  • 48,024
  • 5
  • 125
  • 132
0

Have a look at the using statement. It should be used each time an object is created that implement IDisposable. It ensures the correct dispose of an object, even if an exception is thrown.

http://msdn.microsoft.com/en-us/library/yh598w02.aspx

Darren Young
  • 10,972
  • 36
  • 91
  • 150
  • 1
    Though that is good advice it does not in any way answer the question, which is "why didn't this program crash?" – Eric Lippert Aug 16 '11 at 13:21
  • I tend to have an annoying habit of skimming a question and answering what I think it was asking :) This should have been a comment. – Darren Young Aug 16 '11 at 13:27
  • @Eric: Absolutely, and I'd always suggest using that approach. In fact that's how I found this situation originally, an Icon, being created from a Bitmap, being created from a Stream (or similar), none of which were being disposed. I wasn't sure whether they could be (i.e. does the icon need the underlying bitmap it was constructed with) or not, so tried wrapping them in usings. Then just to double check I wrapped the Icon returned in a using, expecting it to fail and was surprised when all my icons were rendered correctly. – Ian Aug 16 '11 at 14:03