From 33c06d4b35de7e49d07f2b2d60c26fec337a894e Mon Sep 17 00:00:00 2001
From: Fraser Waters <frassle@gmail.com>
Date: Mon, 10 Aug 2015 21:57:05 +0100
Subject: [PATCH] [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.
---
 Source/OpenTK/Platform/Windows/API.cs         |  7 +++
 Source/OpenTK/Platform/Windows/WinGLNative.cs | 55 ++++++++++++++-----
 2 files changed, 48 insertions(+), 14 deletions(-)

diff --git a/Source/OpenTK/Platform/Windows/API.cs b/Source/OpenTK/Platform/Windows/API.cs
index 05d5e73c..c2580c76 100644
--- 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 10460dd0..00a61815 100644
--- 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)
+                                if (icon == IntPtr.Zero)
                                 {
-                                    // Currently using a custom cursor so destroy it 
-                                    // once replaced
-                                    cursor = value;
-                                    cursor_handle = icon;
-                                    oldCursor = Functions.SetCursor(icon);
+                                    throw new System.ComponentModel.Win32Exception();
                                 }
+
+                                // Need to destroy this icon when/if it's replaced by another cursor.
+                                cursor = value;
+                                cursor_handle = icon;
+                                Functions.SetCursor(icon);
+                            }
+                            finally
+                            {
+                                if (success)
+                                {
+                                    // 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);
                     }
                 }
             }