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

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: System.Drawing.Icon.cs

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

Answer 2

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 3

I have had NO END of grief in this area - I've been trying to animate a form's icon (and consequently the one in the task bar) without it leaking resources.

When I disposed of the icon (as suggested on MSDN) resources leaked, when I used "DestroyIcon" all subsequent updates failed. This code below shows everything in the correct order.

API Declaration:

[System.Runtime.InteropServices.DllImport("user32.dll", CharSet = CharSet.Auto)]
extern static bool DestroyIcon(IntPtr handle);

FINALLY the solution:

IntPtr iconHandle = dynamicBitmap.GetHicon();
Icon tempManagedRes = Icon.FromHandle(iconHandle);
this.Icon = (Icon)tempManagedRes.Clone();
tempManagedRes.Dispose();
DestroyIcon(iconHandle);

Also posted in this question: Win32.DestroyIcon vs. Icon.Dispose