[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 05d5e73ccc6832f540ea76f87e902884531b105d..c2580c76809ec7ba45bf44d265eca99353746a25 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 10460dd0b7af3586cecd24c3563a03e5de33fcd0..00a61815e9d8dd1225bcd50170656941fd027659 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);
                     }
                 }
             }