diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CrossLoaderAllocatorHashHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CrossLoaderAllocatorHashHelpers.cs index 9bb4628a3be64f..e2fd4b5d6bb5aa 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CrossLoaderAllocatorHashHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/CrossLoaderAllocatorHashHelpers.cs @@ -17,8 +17,7 @@ internal sealed class LAHashDependentHashTracker ~LAHashDependentHashTracker() { - if (_dependentHandle.IsAllocated) - _dependentHandle.Free(); + _dependentHandle.Dispose(); } } diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index 9dd310326b0ad3..fa9fd638e1ad34 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -212,10 +212,7 @@ internal static SafeSslHandle AllocateSslContext(SslProtocols protocols, SafeX50 } catch { - if (alpnHandle.IsAllocated) - { - alpnHandle.Free(); - } + alpnHandle.Dispose(); throw; } diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs index 29154b77da6325..4669b0d85efd84 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs @@ -326,10 +326,7 @@ protected override void Dispose(bool disposing) _writeBio?.Dispose(); } - if (AlpnHandle.IsAllocated) - { - AlpnHandle.Free(); - } + AlpnHandle.Dispose(); base.Dispose(disposing); } diff --git a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListener.Windows.cs b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListener.Windows.cs index 63c96497909f3a..2c186b9a5da197 100644 --- a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListener.Windows.cs +++ b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListener.Windows.cs @@ -1664,22 +1664,13 @@ private static void SendError(HttpListenerSession session, ulong requestId, Http } finally { - if (headersArrayHandle.IsAllocated) - { - headersArrayHandle.Free(); - } - if (wwwAuthenticateHandle.IsAllocated) - { - wwwAuthenticateHandle.Free(); - } + headersArrayHandle.Dispose(); + wwwAuthenticateHandle.Dispose(); if (challengeHandles != null) { for (int i = 0; i < challengeHandles.Length; i++) { - if (challengeHandles[i].IsAllocated) - { - challengeHandles[i].Free(); - } + challengeHandles[i].Dispose(); } } } diff --git a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListenerResponse.Windows.cs b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListenerResponse.Windows.cs index 0c4b36af8efc75..a5553e6fc20977 100644 --- a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListenerResponse.Windows.cs +++ b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListenerResponse.Windows.cs @@ -584,10 +584,7 @@ private void FreePinnedHeaders(List? pinnedHeaders) { foreach (GCHandle gcHandle in pinnedHeaders) { - if (gcHandle.IsAllocated) - { - gcHandle.Free(); - } + gcHandle.Dispose(); } } } diff --git a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketBuffer.cs b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketBuffer.cs index 712b0d7f39d665..95b696112848d0 100644 --- a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketBuffer.cs +++ b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketBuffer.cs @@ -281,10 +281,7 @@ internal void ReleasePinnedSendBuffer() return; } - if (_pinnedSendBufferHandle.IsAllocated) - { - _pinnedSendBufferHandle.Free(); - } + _pinnedSendBufferHandle.Dispose(); _pinnedSendBuffer = ArraySegment.Empty; } @@ -620,10 +617,7 @@ private bool IsNativeBuffer(IntPtr pBuffer, uint bufferSize) private void CleanUp() { - if (_gcHandle.IsAllocated) - { - _gcHandle.Free(); - } + _gcHandle.Dispose(); ReleasePinnedSendBuffer(); } diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.Windows.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.Windows.cs index b22efcee8d9047..1814b82120b2cc 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.Windows.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.Windows.cs @@ -896,10 +896,7 @@ private void PinSocketAddressBuffer() } // Unpin any existing. - if (_socketAddressGCHandle.IsAllocated) - { - _socketAddressGCHandle.Free(); - } + _socketAddressGCHandle.Dispose(); // Pin down the new one. _socketAddressGCHandle = GCHandle.Alloc(_socketAddress!.Buffer, GCHandleType.Pinned); diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Windows.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Windows.cs index e157c687fd4a1b..08279514c8fd25 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Windows.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Windows.cs @@ -256,10 +256,7 @@ public static SocketError Send(SafeSocketHandle handle, IList { for (int i = 0; i < count; ++i) { - if (objectsToPin[i].IsAllocated) - { - objectsToPin[i].Free(); - } + objectsToPin[i].Dispose(); } if (!useStack) { diff --git a/src/libraries/System.Private.CoreLib/src/System/Buffers/MemoryHandle.cs b/src/libraries/System.Private.CoreLib/src/System/Buffers/MemoryHandle.cs index 2ac65aa35a1264..f510529cb6d84b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/MemoryHandle.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/MemoryHandle.cs @@ -39,10 +39,7 @@ public MemoryHandle(void* pointer, GCHandle handle = default, IPinnable? pinnabl /// public void Dispose() { - if (_handle.IsAllocated) - { - _handle.Free(); - } + _handle.Dispose(); if (_pinnable != null) { diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/GCHandle.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/GCHandle.cs index 579633db55cafc..1af61ae2dcf0fa 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/GCHandle.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/GCHandle.cs @@ -4,7 +4,6 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; -using System.Threading; using Internal.Runtime.CompilerServices; namespace System.Runtime.InteropServices @@ -23,7 +22,7 @@ namespace System.Runtime.InteropServices /// Pinned - same as Normal, but allows the address of the actual object to be taken. /// [StructLayout(LayoutKind.Sequential)] - public partial struct GCHandle + public partial struct GCHandle : IDisposable { // The actual integer handle value that the EE uses internally. private IntPtr _handle; @@ -67,12 +66,25 @@ private GCHandle(object? value, GCHandleType type) /// A new GC handle that protects the object. public static GCHandle Alloc(object? value, GCHandleType type) => new GCHandle(value, type); + /// + public void Dispose() + { + IntPtr handle = _handle; + + if ((nint)handle != 0) + { + _handle = IntPtr.Zero; + + InternalFree(GetHandleValue(handle)); + } + } + /// Frees a GC handle. public void Free() { - // Free the handle if it hasn't already been freed. - IntPtr handle = Interlocked.Exchange(ref _handle, IntPtr.Zero); + IntPtr handle = _handle; ThrowIfInvalid(handle); + _handle = IntPtr.Zero; InternalFree(GetHandleValue(handle)); } diff --git a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/GCHandleTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/GCHandleTests.cs index bf5ec1713cdbc6..a2a559c458c443 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/GCHandleTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/GCHandleTests.cs @@ -119,20 +119,60 @@ public void Free_NotInitialized_ThrowsInvalidOperationException() Assert.Throws(() => handle.Free()); } + [Fact] + public void Dispose_NotInitialized_DoesntThrow() + { + var handle = new GCHandle(); + handle.Dispose(); + } + + [Fact] + public void Dispose_WorksAsExpected() + { + object target = new object(); + GCHandle handle = GCHandle.Alloc(target); + + try + { + Assert.Equal(target, handle.Target); + Assert.True(handle.IsAllocated); + + handle.Dispose(); + + Assert.False(handle.IsAllocated); + + // Repeated Dispose() calls are just a no-op + handle.Dispose(); + handle.Dispose(); + handle.Dispose(); + handle.Dispose(); + + Assert.False(handle.IsAllocated); + } + finally + { + handle.Dispose(); + Assert.False(handle.IsAllocated); + } + } + public static IEnumerable Equals_TestData() { - GCHandle handle = GCHandle.Alloc(new object()); - yield return new object[] { handle, handle, true }; - yield return new object[] { GCHandle.Alloc(new object()), GCHandle.Alloc(new object()), false }; + // xunit disposes of any IDisposables passed as row values in a member data. + // Avoid GCHandle being disposed of automatically by wrapping it. - yield return new object[] { GCHandle.Alloc(new object()), new object(), false }; - yield return new object[] { GCHandle.Alloc(new object()), null, false }; + GCHandle handle = GCHandle.Alloc(new object()); + yield return new object[] { new KeyValuePair(handle, handle), true }; + yield return new object[] { new KeyValuePair(GCHandle.Alloc(new object()), GCHandle.Alloc(new object())), false }; + yield return new object[] { new KeyValuePair(GCHandle.Alloc(new object()), new object()), false }; + yield return new object[] { new KeyValuePair(GCHandle.Alloc(new object()), null), false }; } [Theory] [MemberData(nameof(Equals_TestData))] - public void Equals_Object_ReturnsExpected(GCHandle handle, object other, bool expected) + public void Equals_Object_ReturnsExpected(KeyValuePair pair, bool expected) { + (GCHandle handle, object other) = pair; try { Assert.Equal(expected, handle.Equals(other)); diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index 9e1a56a42a65ad..ade42f0f0220ea 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -13284,7 +13284,7 @@ public sealed partial class FieldOffsetAttribute : System.Attribute public FieldOffsetAttribute(int offset) { } public int Value { get { throw null; } } } - public partial struct GCHandle + public partial struct GCHandle : System.IDisposable { private int _dummyPrimitive; public bool IsAllocated { get { throw null; } } @@ -13292,6 +13292,7 @@ public partial struct GCHandle public System.IntPtr AddrOfPinnedObject() { throw null; } public static System.Runtime.InteropServices.GCHandle Alloc(object? value) { throw null; } public static System.Runtime.InteropServices.GCHandle Alloc(object? value, System.Runtime.InteropServices.GCHandleType type) { throw null; } + public void Dispose() { } public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] object? o) { throw null; } public void Free() { } public static System.Runtime.InteropServices.GCHandle FromIntPtr(System.IntPtr value) { throw null; } diff --git a/src/mono/System.Private.CoreLib/src/System/Threading/Overlapped.cs b/src/mono/System.Private.CoreLib/src/System/Threading/Overlapped.cs index 117e75cef56f63..a46d72d01abfac 100644 --- a/src/mono/System.Private.CoreLib/src/System/Threading/Overlapped.cs +++ b/src/mono/System.Private.CoreLib/src/System/Threading/Overlapped.cs @@ -175,10 +175,7 @@ private void FreeNativeOverlapped() { for (int i = 0; i < _pinnedData.Length; i++) { - if (_pinnedData[i].IsAllocated) - { - _pinnedData[i].Free(); - } + _pinnedData[i].Dispose(); } _pinnedData = null; } @@ -186,8 +183,7 @@ private void FreeNativeOverlapped() if (_pNativeOverlapped != null) { GCHandle handle = *(GCHandle*)(_pNativeOverlapped + 1); - if (handle.IsAllocated) - handle.Free(); + handle.Dispose(); Marshal.FreeHGlobal((IntPtr)_pNativeOverlapped); //CORERT: Interop.MemFree((IntPtr)_pNativeOverlapped);