Dispose Cursor only if it is owned by the creator. (#8865)

* Dispose Cursor only if it owned by application.

* Add check for handle.

* address feedback.

src/System.Windows.Forms.Primitives/tests/UnitTests/Interop/Mocks/MockCursor.cs

@@ -2,7 +2,9 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.ComponentModel;
 using System.Drawing;
+using System.Runtime.InteropServices;
 
 namespace System.Windows.Forms.Primitives.Tests.Interop.Mocks
 {
@@ -18,37 +20,23 @@ internal MockCursor(PCWSTR nResourceId)
             _ownHandle = false;
             _resourceId = nResourceId;
             _handle = PInvoke.LoadCursor(HINSTANCE.Null, nResourceId);
+            if (_handle.IsNull)
+            {
+                throw new Win32Exception(Marshal.GetLastWin32Error());
+            }
         }
 
         public void Dispose()
         {
-            if (!_handle.IsNull)
+            if (!_handle.IsNull && _ownHandle)
             {
-                if (_ownHandle)
-                {
-                    PInvoke.DestroyCursor(_handle);
-                }
-
+                PInvoke.DestroyCursor(_handle);
                 _handle = HCURSOR.Null;
             }
         }
 
-        internal HCURSOR Handle
-        {
-            get
-            {
-                if (_handle.IsNull)
-                {
-                    throw new ObjectDisposedException(nameof(MockCursor));
-                }
-
-                return _handle;
-            }
-        }
+        internal HCURSOR Handle => _handle.IsNull ? throw new ObjectDisposedException(nameof(MockCursor)) : _handle;
 
-        public Size Size
-        {
-            get => SystemInformation.CursorSize;
-        }
+        public Size Size => SystemInformation.CursorSize;
     }
 }

src/System.Windows.Forms/src/Resources/SR.resx

@@ -6961,4 +6961,7 @@ Stack trace where the illegal operation occurred was:
   <data name="InvalidArgumentType" xml:space="preserve">
     <value>Argument '{0}' must be of type '{1}'.</value>
   </data>
+  <data name="FailedToLoadCursor" xml:space="preserve">
+    <value>Failed to load cursor. Error '{1}'.</value>
+  </data>
 </root>

src/System.Windows.Forms/src/System/Windows/Forms/Cursor.cs

@@ -39,6 +39,10 @@ internal unsafe Cursor(PCWSTR nResourceId)
             _ownHandle = false;
             _resourceId = nResourceId;
             _handle = PInvoke.LoadCursor((HINSTANCE)0, nResourceId);
+            if (_handle.IsNull)
+            {
+                throw new Win32Exception(string.Format(SR.FailedToLoadCursor, Marshal.GetLastWin32Error()));
+            }
         }
 
         /// <summary>
@@ -222,13 +226,9 @@ public void Dispose()
 
         private void Dispose(bool disposing)
         {
-            if (!_handle.IsNull)
+            if (!_handle.IsNull && _ownHandle)
             {
-                if (_ownHandle)
-                {
-                    PInvoke.DestroyCursor(_handle);
-                }
-
+                PInvoke.DestroyCursor(_handle);
                 _handle = HCURSOR.Null;
             }
         }
@@ -430,6 +430,11 @@ private unsafe void LoadPicture(IStream.Interface stream, string paramName)
                         picSize.Height,
                         IMAGE_FLAGS.LR_DEFAULTCOLOR).Value;
 
+                    if (_handle.IsNull)
+                    {
+                        throw new Win32Exception(string.Format(SR.FailedToLoadCursor, Marshal.GetLastWin32Error()));
+                    }
+
                     _ownHandle = true;
                 }
                 else
@@ -461,6 +466,8 @@ internal unsafe byte[] GetData()
             return (byte[])_cursorData.Clone();
         }
 
+        internal bool IsValid() => _handle != IntPtr.Zero;
+
         /// <summary>
         ///  Displays the cursor. For every call to Cursor.show() there must have been
         ///  a previous call to Cursor.hide().

src/System.Windows.Forms/src/System/Windows/Forms/Cursors.cs

@@ -65,33 +65,37 @@ public static class Cursors
         public static Cursor WaitCursor => s_wait ??= new Cursor(PInvoke.IDC_WAIT);
 
         public static Cursor Help => s_help ??= new Cursor(PInvoke.IDC_HELP);
+        public static Cursor Hand => s_hand ??= new Cursor(PInvoke.IDC_HAND);
 
-        public static Cursor HSplit => s_hSplit ??= new Cursor(typeof(Cursor), "hsplit.cur");
+        public static Cursor HSplit => GetCursor(ref s_hSplit, "hsplit.cur");
 
-        public static Cursor VSplit => s_vSplit ??= new Cursor(typeof(Cursor), "vsplit.cur");
+        public static Cursor VSplit => GetCursor(ref s_vSplit, "vsplit.cur");
 
-        public static Cursor NoMove2D => s_noMove2D ??= new Cursor(typeof(Cursor), "nomove2d.cur");
+        public static Cursor NoMove2D => GetCursor(ref s_noMove2D, "nomove2d.cur");
 
-        public static Cursor NoMoveHoriz => s_noMoveHoriz ??= new Cursor(typeof(Cursor), "nomoveh.cur");
+        public static Cursor NoMoveHoriz => GetCursor(ref s_noMoveHoriz, "nomoveh.cur");
 
-        public static Cursor NoMoveVert => s_noMoveVert ??= new Cursor(typeof(Cursor), "nomovev.cur");
+        public static Cursor NoMoveVert => GetCursor(ref s_noMoveVert, "nomovev.cur");
 
-        public static Cursor PanEast => s_panEast ??= new Cursor(typeof(Cursor), "east.cur");
+        public static Cursor PanEast => GetCursor(ref s_panEast, "east.cur");
 
-        public static Cursor PanNE => s_panNE ??= new Cursor(typeof(Cursor), "ne.cur");
+        public static Cursor PanNE => GetCursor(ref s_panNE, "ne.cur");
 
-        public static Cursor PanNorth => s_panNorth ??= new Cursor(typeof(Cursor), "north.cur");
+        public static Cursor PanNorth => GetCursor(ref s_panNorth, "north.cur");
 
-        public static Cursor PanNW => s_panNW ??= new Cursor(typeof(Cursor), "nw.cur");
+        public static Cursor PanNW => GetCursor(ref s_panNW, "nw.cur");
 
-        public static Cursor PanSE => s_panSE ??= new Cursor(typeof(Cursor), "se.cur");
+        public static Cursor PanSE => GetCursor(ref s_panSE, "se.cur");
 
-        public static Cursor PanSouth => s_panSouth ??= new Cursor(typeof(Cursor), "south.cur");
+        public static Cursor PanSouth => GetCursor(ref s_panSouth, "south.cur");
 
-        public static Cursor PanSW => s_panSW ??= new Cursor(typeof(Cursor), "sw.cur");
+        public static Cursor PanSW => GetCursor(ref s_panSW, "sw.cur");
 
-        public static Cursor PanWest => s_panWest ??= new Cursor(typeof(Cursor), "west.cur");
+        public static Cursor PanWest => GetCursor(ref s_panWest, "west.cur");
 
-        public static Cursor Hand => s_hand ??= new Cursor(PInvoke.IDC_HAND);
+        private static Cursor GetCursor(ref Cursor? cursor, string resource)
+            => cursor is not null && cursor.IsValid()
+                ? cursor
+                : cursor = new Cursor(typeof(Cursor), resource);
     }
 }

src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/CursorTests.cs

@@ -358,12 +358,8 @@ public void Cursor_Dispose_InvokeNotOwned_Success()
         var cursor = new Cursor(2);
         cursor.Dispose();
 
-        Assert.Throws<ObjectDisposedException>(() => cursor.Handle);
-        Assert.Throws<ObjectDisposedException>(() => cursor.HotSpot);
-
-        cursor.Dispose();
-        Assert.Throws<ObjectDisposedException>(() => cursor.Handle);
-        Assert.Throws<ObjectDisposedException>(() => cursor.HotSpot);
+        // Cursors not owned should not be disposed.
+        Assert.NotEqual(IntPtr.Zero, cursor.Handle);
     }
 
     public static IEnumerable<object[]> Draw_TestData()

src/System.Windows.Forms/tests/UnitTests/System/Windows/Forms/CursorsTests.cs

@@ -22,7 +22,7 @@ public static IEnumerable<object[]> Cursors_TestData()
             yield return new object[] { I(() => Cursors.Default) };
             yield return new object[] { I(() => Cursors.Hand) };
             yield return new object[] { I(() => Cursors.Help) };
-            // yield return new object[] { I(() => Cursors.HSplit) }; Tracking issue: https://github.com/dotnet/winforms/issues/8810.
+            yield return new object[] { I(() => Cursors.HSplit) };
             yield return new object[] { I(() => Cursors.IBeam) };
             yield return new object[] { I(() => Cursors.No) };
             yield return new object[] { I(() => Cursors.NoMove2D) };
@@ -42,7 +42,7 @@ public static IEnumerable<object[]> Cursors_TestData()
             yield return new object[] { I(() => Cursors.SizeNWSE) };
             yield return new object[] { I(() => Cursors.SizeWE) };
             yield return new object[] { I(() => Cursors.UpArrow) };
-            // yield return new object[] { I(() => Cursors.VSplit) }; Tracking issue: https://github.com/dotnet/winforms/issues/8810.
+            yield return new object[] { I(() => Cursors.VSplit) };
             yield return new object[] { I(() => Cursors.WaitCursor) };
         }