Icon.FromHandle: should I Dispose it, or call DestroyIcon?

Question

I use Win32 SHGetFileInfo to get a handle to the icon belonging to a certain file. There are a lot of descriptions how to do this, also on stackoverflow, for instance: Get icons used by shell

After calling the function you have a struct with the handle to the Icon. Using the static method Icon.FromHandle I can convert it to an object of class System.Drawing.Icon. This class implements System.IDisposable. Proper usage would be like:

using (Icon icon = Icon.FromHandle(shFileInfo.hIcon))
{
    // do what you need to do with the icon
}

Upon leaving the using statement the icon object is disposed.

MSDN warns in the description of Icon.FromHandle (click to view):

When using this method, you must dispose of the original icon by using the DestroyIcon method in the Win32 API to ensure that the resources are released.

And in Icon.Dispose (click to view)

Releases all resources used by this Icon.

Question:

Is it enough to Dispose() the object, or should I call both Dispose() and DestroyIcon, or maybe call DestroyIcon instead of Disposing the object?

Answer 1

The .NET Icon class is remarkably clumsy, taking care of this yourself is required. The MSDN article for SHFILEICON makes no bones about it, you must call DestroyIcon(). And so does the MSDN article for Icon.FromHandle(). The exact moment in time you call DestroyIcon matters a great deal as well, it must be delayed until some code either has made a copy of the icon or until you no longer need the icon and would normally call its Dispose() method.

Beware of the code snippet in the MSDN article, it shows a scenario where DestroyIcon() is called early. Okay in that specific case since it is assigned to the Form.Icon property. A corner case and surely not what you want to do.

The only really decent way to deal with this is to override the behavior of Icon.FromHandle() and force the object to take ownership of the native icon handle. So that it will automatically call DestroyIcon() when you dispose it. That requires a hack, the Icon constructor that lets you do this is internal. Reflection to the rescue, you can use the code from this post, note the GetConstructor() call. Start feeling good about it by writing a little unit test that does this a million times. If you hate it then write your own IDisposable wrapper so you can both dispose the icon and pinvoke DestroyIcon().

Answer 2

Addition by OP. There is an error in this answer. Because of all the comments it became harsh to see the forest through the trees. Hence I decided to edit this answer. (Sorry if I offended someone)

The .net source code is online: http://referencesource.microsoft.com/#System.Drawing/commonui/System/Drawing/Icon.cs,81a28d20524554ae

Take a look at Icon.FromHandle:

public static Icon FromHandle(IntPtr handle)
{
    IntSecurity.ObjectFromWin32Handle.Demand();
    return new Icon(handle);
}
internal Icon(IntPtr handle) : this(handle, false)
{
}
internal Icon(IntPtr handle, bool takeOwnership)
{
    if (handle == IntPtr.Zero)
    {
        throw new ArgumentException(SR.GetString(SR.InvalidGDIHandle,
              (typeof(Icon)).Name));
    }
    this.handle = handle;
    this.ownHandle = takeOwnership;
}

Note that after Icon.FromHandle ownHandle is false.

Let's look at Dispose:

void Dispose(bool disposing)
{
    if (handle != IntPtr.Zero)
    {
        DestroyHandle();
    }
}

internal void DestroyHandle()
{
    if (ownHandle)
    {
        SafeNativeMethods.DestroyIcon(new HandleRef(this, handle));
        handle = IntPtr.Zero;
    }
}

Conclusion: After Icon.FromHandle, the field ownHandle is false, and thus Dispose / FromHandle won't call DestroyIcon

Therefore: if you create an Icon using Icon.FromHandle you'll have to Dispose() the Icon as well as call DestroyIcon, just as the remarks section says