Move System.Drawing.Common to SafeHandles et. al.

src/libraries/Common/src/Interop/Windows/Gdi32/Interop.DeleteObject.cs

@@ -11,6 +11,23 @@ internal static partial class Gdi32
         [GeneratedDllImport(Libraries.Gdi32)]
         public static partial bool DeleteObject(IntPtr ho);
 
+        public static bool DeleteObject(IntPtr ho, SafeHandle owningHandle)
+        {
+            bool releaseOwner = false;
+            try
+            {
+                owningHandle.DangerousAddRef(ref releaseOwner);
+                return DeleteObject(ho);
+            }
+            finally
+            {
+                if (releaseOwner)
+                {
+                    owningHandle.DangerousRelease();
+                }
+            }
+        }
+
         public static bool DeleteObject(HandleRef ho)
         {
             bool result = DeleteObject(ho.Handle);

src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs

@@ -166,6 +166,13 @@ public static async Task<T> ThrowsAsync<T>(string expectedParamName, Func<Task>
             return Throws(typeof(TNetCoreExceptionType), typeof(TNetFxExceptionType), action);
         }
 
+        public static Exception Throws<TNetCoreExceptionType, TNetFxExceptionType>(Func<object> action)
+            where TNetCoreExceptionType : Exception
+            where TNetFxExceptionType : Exception
+        {
+            return Throws(typeof(TNetCoreExceptionType), typeof(TNetFxExceptionType), action);
+        }
+
         public static Exception Throws(Type netCoreExceptionType, Type netFxExceptionType, Action action)
         {
             if (IsNetFramework)
@@ -178,6 +185,18 @@ public static Exception Throws(Type netCoreExceptionType, Type netFxExceptionTyp
             }
         }
 
+        public static Exception Throws(Type netCoreExceptionType, Type netFxExceptionType, Func<object> action)
+        {
+            if (IsNetFramework)
+            {
+                return Assert.Throws(netFxExceptionType, action);
+            }
+            else
+            {
+                return Assert.Throws(netCoreExceptionType, action);
+            }
+        }
+
         public static void Throws<TNetCoreExceptionType, TNetFxExceptionType>(string netCoreParamName, string netFxParamName, Action action)
             where TNetCoreExceptionType : ArgumentException
             where TNetFxExceptionType : ArgumentException

src/libraries/System.Drawing.Common/src/Microsoft/Win32/SafeHandles/SafeBrushHandle.cs

@@ -0,0 +1,22 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using Gdip = System.Drawing.SafeNativeMethods.Gdip;
+
+namespace Microsoft.Win32.SafeHandles
+{
+    internal class SafeBrushHandle : SafeGdiPlusHandle
+    {
+        public SafeBrushHandle(IntPtr preexistingHandle, bool ownsHandle) : base(ownsHandle)
+        {
+            SetHandle(preexistingHandle);
+        }
+
+        public SafeBrushHandle() : base(true)
+        {
+        }
+
+        protected override int ReleaseHandleImpl() => Gdip.GdipDeleteBrush(handle);
+    }
+}

src/libraries/System.Drawing.Common/src/Microsoft/Win32/SafeHandles/SafeGdiPlusHandle.cs

@@ -0,0 +1,41 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Diagnostics;
+using System.Drawing;
+using System.Runtime.InteropServices;
+using Gdip = System.Drawing.SafeNativeMethods.Gdip;
+
+namespace Microsoft.Win32.SafeHandles
+{
+    internal abstract class SafeGdiPlusHandle : SafeHandle
+    {
+        public SafeGdiPlusHandle(bool ownsHandle) : base(IntPtr.Zero, ownsHandle)
+        {
+        }
+
+        public override bool IsInvalid => handle == IntPtr.Zero;
+
+        protected abstract int ReleaseHandleImpl();
+
+        protected override bool ReleaseHandle()
+        {
+            int releaseResult = Gdip.Ok;
+            try
+            {
+                if (!Gdip.Initialized)
+                {
+                    releaseResult = ReleaseHandleImpl();
+                }
+                Debug.Assert(releaseResult == Gdip.Ok, $"GDI+ returned an error status: {releaseResult}");
+            }
+            catch (Exception ex) when (!ClientUtils.IsSecurityOrCriticalException(ex))
+            {
+                // Catch all non fatal exceptions. This includes exceptions like EntryPointNotFoundException, that is thrown
+                // on Windows Nano.
+            }
+            return releaseResult == Gdip.Ok;
+        }
+    }
+}

src/libraries/System.Drawing.Common/src/Microsoft/Win32/SafeHandles/SafeGraphicsPathHandle.cs

@@ -0,0 +1,22 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using Gdip = System.Drawing.SafeNativeMethods.Gdip;
+
+namespace Microsoft.Win32.SafeHandles
+{
+    internal class SafeGraphicsPathHandle : SafeGdiPlusHandle
+    {
+        public SafeGraphicsPathHandle(IntPtr preexistingHandle, bool ownsHandle) : base(ownsHandle)
+        {
+            SetHandle(preexistingHandle);
+        }
+
+        public SafeGraphicsPathHandle() : base(true)
+        {
+        }
+
+        protected override int ReleaseHandleImpl() => Gdip.GdipDeletePath(handle);
+    }
+}

src/libraries/System.Drawing.Common/src/Microsoft/Win32/SafeHandles/SafeMatrixHandle.cs

@@ -0,0 +1,25 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using Gdip = System.Drawing.SafeNativeMethods.Gdip;
+
+namespace Microsoft.Win32.SafeHandles
+{
+    internal class SafeMatrixHandle : SafeGdiPlusHandle
+    {
+        public SafeMatrixHandle(IntPtr preexistingHandle, bool ownsHandle) : base(ownsHandle)
+        {
+            SetHandle(preexistingHandle);
+        }
+
+        public SafeMatrixHandle() : base(true)
+        {
+        }
+
+        protected override int ReleaseHandleImpl() => Gdip.GdipDeleteMatrix(handle);
+
+        internal bool HasEqualHandle(SafeMatrixHandle other) =>
+            DangerousGetHandle() == other.DangerousGetHandle();
+    }
+}

src/libraries/System.Drawing.Common/src/Microsoft/Win32/SafeHandles/SafePenHandle.cs

@@ -0,0 +1,22 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using Gdip = System.Drawing.SafeNativeMethods.Gdip;
+
+namespace Microsoft.Win32.SafeHandles
+{
+    internal class SafePenHandle : SafeGdiPlusHandle
+    {
+        public SafePenHandle(IntPtr preexistingHandle, bool ownsHandle) : base(ownsHandle)
+        {
+            SetHandle(preexistingHandle);
+        }
+
+        public SafePenHandle() : base(true)
+        {
+        }
+
+        protected override int ReleaseHandleImpl() => Gdip.GdipDeletePen(handle);
+    }
+}

src/libraries/System.Drawing.Common/src/Microsoft/Win32/SafeHandles/SafeRegionHandle.cs

@@ -0,0 +1,22 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using Gdip = System.Drawing.SafeNativeMethods.Gdip;
+
+namespace Microsoft.Win32.SafeHandles
+{
+    internal class SafeRegionHandle : SafeGdiPlusHandle
+    {
+        public SafeRegionHandle(IntPtr preexistingHandle, bool ownsHandle) : base(ownsHandle)
+        {
+            SetHandle(preexistingHandle);
+        }
+
+        public SafeRegionHandle() : base(true)
+        {
+        }
+
+        protected override int ReleaseHandleImpl() => Gdip.GdipDeleteRegion(handle);
+    }
+}

src/libraries/System.Drawing.Common/src/System.Drawing.Common.csproj

@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <DefineConstants>$(DefineConstants);DRAWING_NAMESPACE</DefineConstants>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
@@ -31,6 +31,12 @@ Unix support is disabled by default. See https://aka.ms/systemdrawingnonwindows
     <Compile Include="$(CoreLibSharedDir)System\Diagnostics\CodeAnalysis\UnconditionalSuppressMessageAttribute.cs" />
   </ItemGroup>
   <ItemGroup Condition="'$(TargetsAnyOS)' != 'true'">
+    <Compile Include="Microsoft\Win32\SafeHandles\SafeBrushHandle.cs" />
+    <Compile Include="Microsoft\Win32\SafeHandles\SafeGdiPlusHandle.cs" />
+    <Compile Include="Microsoft\Win32\SafeHandles\SafeGraphicsPathHandle.cs" />
+    <Compile Include="Microsoft\Win32\SafeHandles\SafeMatrixHandle.cs" />
+    <Compile Include="Microsoft\Win32\SafeHandles\SafePenHandle.cs" />
+    <Compile Include="Microsoft\Win32\SafeHandles\SafeRegionHandle.cs" />
     <Compile Include="System\Drawing\Bitmap.cs" />
     <Compile Include="System\Drawing\BitmapSuffixInSameAssemblyAttribute.cs" />
     <Compile Include="System\Drawing\BitmapSuffixInSatelliteAssemblyAttribute.cs" />
@@ -41,6 +47,7 @@ Unix support is disabled by default. See https://aka.ms/systemdrawingnonwindows
     <Compile Include="System\Drawing\ContentAlignment.cs" />
     <Compile Include="System\Drawing\IDeviceContext.cs" />
     <Compile Include="System\Drawing\GdiplusNative.cs" />
+    <Compile Include="System\Drawing\GdiplusNative.ManualMarshalling.cs" />
     <Compile Include="System\Drawing\Graphics.cs" />
     <Compile Include="System\Drawing\GraphicsUnit.cs" />
     <Compile Include="System\Drawing\IconConverter.cs" />
@@ -79,6 +86,7 @@ Unix support is disabled by default. See https://aka.ms/systemdrawingnonwindows
     <Compile Include="System\Drawing\Drawing2D\FillMode.cs" />
     <Compile Include="System\Drawing\Drawing2D\FlushIntention.cs" />
     <Compile Include="System\Drawing\Drawing2D\GraphicsContainer.cs" />
+    <Compile Include="System\Drawing\Drawing2D\GraphicsPath.cs" />
     <Compile Include="System\Drawing\Drawing2D\HatchBrush.cs" />
     <Compile Include="System\Drawing\Drawing2D\HatchStyle.cs" />
     <Compile Include="System\Drawing\Drawing2D\InterpolationMode.cs" />

src/libraries/System.Drawing.Common/src/System/Drawing/Brush.cs

@@ -1,67 +1,49 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using Microsoft.Win32.SafeHandles;
 using System.ComponentModel;
-using System.Diagnostics;
-using System.Globalization;
-using System.Runtime.InteropServices;
-using Gdip = System.Drawing.SafeNativeMethods.Gdip;
+using System.Threading;
 
 namespace System.Drawing
 {
     public abstract class Brush : MarshalByRefObject, ICloneable, IDisposable
     {
-#if FINALIZATION_WATCH
-        private string allocationSite = Graphics.GetAllocationStack();
-#endif
         // Handle to native GDI+ brush object to be used on demand.
-        private IntPtr _nativeBrush;
+        private SafeBrushHandle _nativeBrush = null!;
 
         public abstract object Clone();
 
-        protected internal void SetNativeBrush(IntPtr brush) => SetNativeBrushInternal(brush);
-        internal void SetNativeBrushInternal(IntPtr brush) => _nativeBrush = brush;
+        protected internal void SetNativeBrush(IntPtr brush) => SetNativeBrushInternal(new(brush, true));
+
+        internal void SetNativeBrushInternal(SafeBrushHandle brush)
+        {
+            Interlocked.Exchange(ref _nativeBrush, brush)?.SetHandleAsInvalid();
+        }
 
         [Browsable(false), EditorBrowsable(EditorBrowsableState.Never)]
-        internal IntPtr NativeBrush => _nativeBrush;
+        internal SafeBrushHandle SafeBrushHandle => _nativeBrush;
+
+        protected Brush()
+        {
+        }
+
+        private protected Brush(SafeBrushHandle nativeBrush)
+        {
+            _nativeBrush = nativeBrush;
+        }
 
         public void Dispose()
         {
             Dispose(true);
-            GC.SuppressFinalize(this);
         }
 
         protected virtual void Dispose(bool disposing)
         {
-#if FINALIZATION_WATCH
-            if (!disposing && nativeBrush != IntPtr.Zero )
-                Debug.WriteLine("**********************\nDisposed through finalization:\n" + allocationSite);
-#endif
-
-            if (_nativeBrush != IntPtr.Zero)
+            if (disposing && _nativeBrush != null)
             {
-                try
-                {
-#if DEBUG
-                    int status = !Gdip.Initialized ? Gdip.Ok :
-#endif
-                    Gdip.GdipDeleteBrush(new HandleRef(this, _nativeBrush));
-#if DEBUG
-                    Debug.Assert(status == Gdip.Ok, $"GDI+ returned an error status: {status.ToString(CultureInfo.InvariantCulture)}");
-#endif
-                }
-                catch (Exception ex) when (!ClientUtils.IsSecurityOrCriticalException(ex))
-                {
-                    // Catch all non fatal exceptions. This includes exceptions like EntryPointNotFoundException, that is thrown
-                    // on Windows Nano.
-                }
-                finally
-                {
-                    _nativeBrush = IntPtr.Zero;
-                }
+                _nativeBrush.Dispose();
             }
         }
-
-        ~Brush() => Dispose(false);
     }
 }

src/libraries/System.Drawing.Common/src/System/Drawing/Drawing2D/CustomLineCap.cs

@@ -28,8 +28,8 @@ public CustomLineCap(GraphicsPath? fillPath, GraphicsPath? strokePath, LineCap b
         {
             IntPtr nativeLineCap;
             int status = Gdip.GdipCreateCustomLineCap(
-                                new HandleRef(fillPath, (fillPath == null) ? IntPtr.Zero : fillPath._nativePath),
-                                new HandleRef(strokePath, (strokePath == null) ? IntPtr.Zero : strokePath._nativePath),
+                                fillPath?.SafeGraphicsPathHandle,
+                                strokePath?.SafeGraphicsPathHandle,
                                 baseCap, baseInset, out nativeLineCap);
 
             if (status != Gdip.Ok)