Merge pull request opentk#289 from Frassle/leak

[Win32] Fix leaks in Win32 Cursor property
luxkun · Aug 10, 2015 · 3026154 · 3026154
2 parents 8e7d66a + 33c06d4
commit 3026154
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 13 deletions.
diff --git a/Source/OpenTK/Platform/Windows/API.cs b/Source/OpenTK/Platform/Windows/API.cs
@@ -1739,6 +1739,13 @@ private static IntPtr RawInputAlign(IntPtr data)
 
         #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
@@ -1127,16 +1127,21 @@ public override MouseCursor Cursor
             }
             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 @@ public override MouseCursor Cursor
                             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);
                     }
                 }
             }