[Win32] Fix leaks in Win32 Cursor property

GetIconInfo
(https://msdn.microsoft.com/en-us/library/windows/desktop/ms648070(v=vs.85).aspx)
creates bitmaps that must be deleted after the call to
CreateIconIndirect, which copies the bitmaps to the icon it creates.

bmpIcon created by Bitmap.GetHicon is now destroyed after being used.

The return value of SetCursor was used to retrieve the last cursor, as
it could have been set by another library (Some UI libraries change the
cursor using the .net Cursor) this could of leaked the cursor we created
and now lost track of. We now delete the handle we had set, not the one
returned by SetCursor.

diff --git a/Source/OpenTK/Platform/Windows/API.cs b/Source/OpenTK/Platform/Windows/API.cs
index 05d5e73..c2580c7 100644 (file)
--- a/Source/OpenTK/Platform/Windows/API.cs
+++ b/Source/OpenTK/Platform/Windows/API.cs
@@ -1739,6 +1739,13 @@ namespace OpenTK.Platform.Windows
 
         #endregion
 
+        #region DeleteObject
+
+        [DllImport("gdi32.dll", SetLastError = true)]
+        internal static extern BOOL DeleteObject([In]IntPtr hObject);
+        
+        #endregion
+
         #endregion
 
         #region Timer Functions

diff --git a/Source/OpenTK/Platform/Windows/WinGLNative.cs b/Source/OpenTK/Platform/Windows/WinGLNative.cs
index 10460dd..00a6181 100644 (file)
--- a/Source/OpenTK/Platform/Windows/WinGLNative.cs
+++ b/Source/OpenTK/Platform/Windows/WinGLNative.cs
@@ -1127,16 +1127,21 @@ namespace OpenTK.Platform.Windows
             }
             set
             {
+                if (value == null)
+                {
+                    throw new ArgumentNullException("value");
+                }
+
                 if (value != cursor)
                 {
-                    bool destoryOld = cursor != MouseCursor.Default;
-                    IntPtr oldCursor = IntPtr.Zero;
+                    MouseCursor oldCursor = cursor;
+                    IntPtr oldCursorHandle = cursor_handle;
 
                     if (value == MouseCursor.Default)
                     {
-                        cursor_handle = Functions.LoadCursor(CursorName.Arrow);
-                        oldCursor = Functions.SetCursor(cursor_handle);
                         cursor = value;
+                        cursor_handle = Functions.LoadCursor(CursorName.Arrow);
+                        Functions.SetCursor(cursor_handle);
                     }
                     else
                     {
@@ -1159,29 +1164,51 @@ namespace OpenTK.Platform.Windows
                             var bmpIcon = bmp.GetHicon();
                             var success = Functions.GetIconInfo(bmpIcon, out iconInfo);
 
-                            if (success)
+                            try
                             {
+                                if (!success)
+                                {
+                                    throw new System.ComponentModel.Win32Exception();
+                                }
+
                                 iconInfo.xHotspot = value.X;
                                 iconInfo.yHotspot = value.Y;
                                 iconInfo.fIcon = false;
 
                                 var icon = Functions.CreateIconIndirect(ref iconInfo);
+                                if (icon == IntPtr.Zero)
+                                {
+                                    throw new System.ComponentModel.Win32Exception();
+                                }
 
-                                if (icon != IntPtr.Zero)
+                                // Need to destroy this icon when/if it's replaced by another cursor.
+                                cursor = value;
+                                cursor_handle = icon;
+                                Functions.SetCursor(icon);
+                            }
+                            finally
+                            {
+                                if (success)
                                 {
-                                    // Currently using a custom cursor so destroy it 
-                                    // once replaced
-                                    cursor = value;
-                                    cursor_handle = icon;
-                                    oldCursor = Functions.SetCursor(icon);
+                                    // GetIconInfo creates bitmaps for the hbmMask and hbmColor members of ICONINFO. 
+                                    // The calling application must manage these bitmaps and delete them when they are no longer necessary.
+                                    Functions.DeleteObject(iconInfo.hbmColor);
+                                    Functions.DeleteObject(iconInfo.hbmMask);
                                 }
+
+                                Functions.DestroyIcon(bmpIcon);
                             }
                         }
                     }
+                    
+                    Debug.Assert(oldCursorHandle != IntPtr.Zero);
+                    Debug.Assert(oldCursorHandle != cursor_handle);
+                    Debug.Assert(oldCursor != cursor);
 
-                    if (destoryOld && oldCursor != IntPtr.Zero)
+                    // If we've replaced a custom (non-default) cursor we need to free the handle.
+                    if (oldCursor != MouseCursor.Default)
                     {
-                        Functions.DestroyIcon(oldCursor);
+                        Functions.DestroyIcon(oldCursorHandle);
                     }
                 }
             }