From e31d807df21d8b2adf537641623c1f03688e682c Mon Sep 17 00:00:00 2001 From: JeffCyr Date: Mon, 5 Feb 2018 09:34:44 -0500 Subject: [PATCH 01/10] Modified Dns.GetHostAddressesAsync to be truly async --- .../Interop/Windows/Winsock/AddressInfoEx.cs | 21 ++ .../Windows/Winsock/Interop.GetAddrInfoExW.cs | 36 ++ .../src/System.Net.NameResolution.csproj | 24 +- .../src/System/Net/DNS.cs | 58 ++- .../src/System/Net/DnsResolveAsyncResult.cs | 25 ++ .../src/System/Net/NameResolutionPal.Unix.cs | 7 + .../System/Net/NameResolutionPal.Windows.cs | 333 ++++++++++++++++-- .../PalTests/Fakes/FakeContextAwareResult.cs | 91 +++++ ...System.Net.NameResolution.Pal.Tests.csproj | 24 +- .../UnitTests/Fakes/FakeNameResolutionPal.cs | 7 + ...ystem.Net.NameResolution.Unit.Tests.csproj | 3 + 11 files changed, 566 insertions(+), 63 deletions(-) create mode 100644 src/Common/src/Interop/Windows/Winsock/AddressInfoEx.cs create mode 100644 src/Common/src/Interop/Windows/Winsock/Interop.GetAddrInfoExW.cs create mode 100644 src/System.Net.NameResolution/src/System/Net/DnsResolveAsyncResult.cs create mode 100644 src/System.Net.NameResolution/tests/PalTests/Fakes/FakeContextAwareResult.cs diff --git a/src/Common/src/Interop/Windows/Winsock/AddressInfoEx.cs b/src/Common/src/Interop/Windows/Winsock/AddressInfoEx.cs new file mode 100644 index 000000000000..a50512058afa --- /dev/null +++ b/src/Common/src/Interop/Windows/Winsock/AddressInfoEx.cs @@ -0,0 +1,21 @@ +using System.Net.Internals; +using System.Runtime.InteropServices; + +namespace System.Net.Sockets +{ + [StructLayout(LayoutKind.Sequential)] + internal unsafe struct AddressInfoEx + { + internal AddressInfoHints ai_flags; + internal AddressFamily ai_family; + internal SocketType ai_socktype; + internal ProtocolFamily ai_protocol; + internal int ai_addrlen; + internal IntPtr ai_canonname; // Ptr to the canonical name - check for NULL + internal byte* ai_addr; // Ptr to the sockaddr structure + internal IntPtr ai_blob; // Unused ptr to blob data about provider + internal int ai_bloblen; + internal IntPtr ai_provider; // Unused ptr to the namespace provider guid + internal AddressInfoEx* ai_next; // Next structure in linked list + } +} diff --git a/src/Common/src/Interop/Windows/Winsock/Interop.GetAddrInfoExW.cs b/src/Common/src/Interop/Windows/Winsock/Interop.GetAddrInfoExW.cs new file mode 100644 index 000000000000..29a137685825 --- /dev/null +++ b/src/Common/src/Interop/Windows/Winsock/Interop.GetAddrInfoExW.cs @@ -0,0 +1,36 @@ +using System; +using System.Net.Sockets; +using System.Runtime.ConstrainedExecution; +using System.Runtime.InteropServices; +using System.Threading; + +internal static partial class Interop +{ + internal static partial class Winsock + { + internal unsafe delegate void GetAddrInfoExCompletionCallback([In] int error, [In] int bytes, [In] NativeOverlapped* overlapped); + + [DllImport(Interop.Libraries.Ws2_32, ExactSpelling = true, CharSet = CharSet.Unicode, BestFitMapping = false, ThrowOnUnmappableChar = true, SetLastError = true)] + internal static extern unsafe int GetAddrInfoExW( + [In] string name, + [In] string serviceName, + [In] int namespaceId, + [In] IntPtr providerId, + [In] ref AddressInfoEx hints, + [Out] out AddressInfoEx* result, + [In] IntPtr timeout, + [In] ref NativeOverlapped overlapped, + [In] GetAddrInfoExCompletionCallback callback, + [Out] out IntPtr cancelHandle + ); + + [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] + [DllImport("ws2_32.dll", ExactSpelling = true, SetLastError = true)] + internal static extern unsafe void FreeAddrInfoEx([In] AddressInfoEx* addressInfo); + + [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] + [DllImport("ws2_32.dll", ExactSpelling = true, SetLastError = true)] + internal static extern int GetAddrInfoExCancel([In] ref IntPtr cancelHandle); + } +} + diff --git a/src/System.Net.NameResolution/src/System.Net.NameResolution.csproj b/src/System.Net.NameResolution/src/System.Net.NameResolution.csproj index ef34f9675be7..54c6db45fcf9 100644 --- a/src/System.Net.NameResolution/src/System.Net.NameResolution.csproj +++ b/src/System.Net.NameResolution/src/System.Net.NameResolution.csproj @@ -5,7 +5,7 @@ System.Net.NameResolution {1714448C-211E-48C1-8B7E-4EE667D336A1} true - + @@ -46,6 +46,7 @@ Common\System\Net\IPEndPointStatics.cs + @@ -118,6 +119,24 @@ Interop\Windows\Winsock\SafeFreeAddrInfo.cs + + Interop\Windows\Winsock\AddressInfoEx.cs + + + Interop\Windows\Winsock\Interop.GetAddrInfoExW.cs + + + Common\Microsoft\Win32\SafeHandles\SafeLibraryHandle.cs + + + Interop\Windows\Kernel32\Interop.GetProcAddress.cs + + + Interop\Windows\Kernel32\Interop.LoadLibraryEx.cs + + + Interop\Windows\Kernel32\Interop.FreeLibrary.cs + @@ -189,7 +208,8 @@ + - + \ No newline at end of file diff --git a/src/System.Net.NameResolution/src/System/Net/DNS.cs b/src/System.Net.NameResolution/src/System/Net/DNS.cs index a51ca67828c1..00c07be09af1 100644 --- a/src/System.Net.NameResolution/src/System/Net/DNS.cs +++ b/src/System.Net.NameResolution/src/System/Net/DNS.cs @@ -252,32 +252,9 @@ public static IPHostEntry Resolve(string hostName) return ipHostEntry; } - private class ResolveAsyncResult : ContextAwareResult - { - // Forward lookup - internal ResolveAsyncResult(string hostName, object myObject, bool includeIPv6, object myState, AsyncCallback myCallBack) : - base(myObject, myState, myCallBack) - { - this.hostName = hostName; - this.includeIPv6 = includeIPv6; - } - - // Reverse lookup - internal ResolveAsyncResult(IPAddress address, object myObject, bool includeIPv6, object myState, AsyncCallback myCallBack) : - base(myObject, myState, myCallBack) - { - this.includeIPv6 = includeIPv6; - this.address = address; - } - - internal readonly string hostName; - internal bool includeIPv6; - internal IPAddress address; - } - private static void ResolveCallback(object context) { - ResolveAsyncResult result = (ResolveAsyncResult)context; + DnsResolveAsyncResult result = (DnsResolveAsyncResult)context; IPHostEntry hostEntry; try { @@ -316,7 +293,7 @@ private static IAsyncResult HostResolutionBeginHelper(string hostName, bool just // See if it's an IP Address. IPAddress address; - ResolveAsyncResult asyncResult; + DnsResolveAsyncResult asyncResult; if (IPAddress.TryParse(hostName, out address)) { if (throwOnIIPAny && (address.Equals(IPAddress.Any) || address.Equals(IPAddress.IPv6Any))) @@ -324,7 +301,7 @@ private static IAsyncResult HostResolutionBeginHelper(string hostName, bool just throw new ArgumentException(SR.net_invalid_ip_addr, nameof(hostName)); } - asyncResult = new ResolveAsyncResult(address, null, includeIPv6, state, requestCallback); + asyncResult = new DnsResolveAsyncResult(address, null, includeIPv6, state, requestCallback); if (justReturnParsedIp) { @@ -337,19 +314,26 @@ private static IAsyncResult HostResolutionBeginHelper(string hostName, bool just } else { - asyncResult = new ResolveAsyncResult(hostName, null, includeIPv6, state, requestCallback); + asyncResult = new DnsResolveAsyncResult(hostName, null, includeIPv6, state, requestCallback); } // Set up the context, possibly flow. asyncResult.StartPostingAsyncOp(false); - // Start the resolve. - Task.Factory.StartNew( - s => ResolveCallback(s), - asyncResult, - CancellationToken.None, - TaskCreationOptions.DenyChildAttach, - TaskScheduler.Default); + if (NameResolutionPal.SupportsGetAddrInfoAsync && includeIPv6 && SocketProtocolSupportPal.OSSupportsIPv6 && address == null) + { + NameResolutionPal.GetAddrInfoAsync(hostName, asyncResult); + } + else + { + // Start the resolve. + Task.Factory.StartNew( + s => ResolveCallback(s), + asyncResult, + CancellationToken.None, + TaskCreationOptions.DenyChildAttach, + TaskScheduler.Default); + } // Finish the flowing, maybe it completed? This does nothing if we didn't initiate the flowing above. asyncResult.FinishPostingAsyncOp(); @@ -371,7 +355,7 @@ private static IAsyncResult HostResolutionBeginHelper(IPAddress address, bool fl if (NetEventSource.IsEnabled) NetEventSource.Info(null, address); // Set up the context, possibly flow. - ResolveAsyncResult asyncResult = new ResolveAsyncResult(address, null, includeIPv6, state, requestCallback); + DnsResolveAsyncResult asyncResult = new DnsResolveAsyncResult(address, null, includeIPv6, state, requestCallback); if (flowContext) { asyncResult.StartPostingAsyncOp(false); @@ -399,7 +383,7 @@ private static IPHostEntry HostResolutionEndHelper(IAsyncResult asyncResult) { throw new ArgumentNullException(nameof(asyncResult)); } - ResolveAsyncResult castedResult = asyncResult as ResolveAsyncResult; + DnsResolveAsyncResult castedResult = asyncResult as DnsResolveAsyncResult; if (castedResult == null) { throw new ArgumentException(SR.net_io_invalidasyncresult, nameof(asyncResult)); @@ -611,7 +595,7 @@ public static IPHostEntry EndResolve(IAsyncResult asyncResult) } catch (SocketException ex) { - IPAddress address = ((ResolveAsyncResult)asyncResult).address; + IPAddress address = ((DnsResolveAsyncResult)asyncResult).address; if (address == null) throw; // BeginResolve was called with a HostName, not an IPAddress diff --git a/src/System.Net.NameResolution/src/System/Net/DnsResolveAsyncResult.cs b/src/System.Net.NameResolution/src/System/Net/DnsResolveAsyncResult.cs new file mode 100644 index 000000000000..56dd163034dc --- /dev/null +++ b/src/System.Net.NameResolution/src/System/Net/DnsResolveAsyncResult.cs @@ -0,0 +1,25 @@ +namespace System.Net +{ + internal class DnsResolveAsyncResult : ContextAwareResult + { + // Forward lookup + internal DnsResolveAsyncResult(string hostName, object myObject, bool includeIPv6, object myState, AsyncCallback myCallBack) : + base(myObject, myState, myCallBack) + { + this.hostName = hostName; + this.includeIPv6 = includeIPv6; + } + + // Reverse lookup + internal DnsResolveAsyncResult(IPAddress address, object myObject, bool includeIPv6, object myState, AsyncCallback myCallBack) : + base(myObject, myState, myCallBack) + { + this.includeIPv6 = includeIPv6; + this.address = address; + } + + internal readonly string hostName; + internal bool includeIPv6; + internal IPAddress address; + } +} \ No newline at end of file diff --git a/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Unix.cs b/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Unix.cs index 139e2740f9fd..8ad4c869dc27 100644 --- a/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Unix.cs +++ b/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Unix.cs @@ -13,6 +13,8 @@ namespace System.Net { internal static partial class NameResolutionPal { + public static bool SupportsGetAddrInfoAsync => false; + private static SocketError GetSocketErrorForErrno(int errno) { switch (errno) @@ -194,6 +196,11 @@ public static unsafe SocketError TryGetAddrInfo(string name, out IPHostEntry hos return SocketError.Success; } + internal static void GetAddrInfoAsync(string name, Dns.ResolveAsyncResult asyncResult) + { + throw new NotSupportedException(); + } + public static unsafe string TryGetNameInfo(IPAddress addr, out SocketError socketError, out int nativeErrorCode) { byte* buffer = stackalloc byte[Interop.Sys.NI_MAXHOST + 1 /*for null*/]; diff --git a/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Windows.cs b/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Windows.cs index 36a69f8637aa..2f05b3ba8a6f 100644 --- a/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Windows.cs +++ b/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Windows.cs @@ -4,9 +4,12 @@ using System.Collections.Generic; using System.Net.Sockets; +using System.Runtime.ConstrainedExecution; using System.Runtime.InteropServices; using System.Text; using System.Threading; +using System.Threading.Tasks; +using Microsoft.Win32.SafeHandles; using ProtocolFamily = System.Net.Internals.ProtocolFamily; namespace System.Net @@ -20,6 +23,18 @@ internal static class NameResolutionPal private static bool s_initialized; private static readonly object s_initializedLock = new object(); + private static readonly unsafe Interop.Winsock.GetAddrInfoExCompletionCallback s_getAddrInfoExCallback = GetAddressInfoExCallback; + private static bool s_getAddrInfoExSupported; + + public static bool SupportsGetAddrInfoAsync + { + get + { + EnsureSocketsAreInitialized(); + return s_getAddrInfoExSupported; + } + } + /*++ Routine Description: @@ -232,7 +247,6 @@ public static unsafe SocketError TryGetAddrInfo(string name, out IPHostEntry hos // while (pAddressInfo != null) { - SocketAddress sockaddr; // // Retrieve the canonical name for the host - only appears in the first AddressInfo // entry in the returned array. @@ -247,29 +261,13 @@ public static unsafe SocketError TryGetAddrInfo(string name, out IPHostEntry hos // We also filter based on whether IPv6 is supported on the current // platform / machine. // - if ((pAddressInfo->ai_family == AddressFamily.InterNetwork) || // Never filter v4 - (pAddressInfo->ai_family == AddressFamily.InterNetworkV6 && SocketProtocolSupportPal.OSSupportsIPv6)) + if (pAddressInfo->ai_family == AddressFamily.InterNetwork) { - sockaddr = new SocketAddress(pAddressInfo->ai_family, pAddressInfo->ai_addrlen); - // - // Push address data into the socket address buffer - // - for (int d = 0; d < pAddressInfo->ai_addrlen; d++) - { - sockaddr[d] = *(pAddressInfo->ai_addr + d); - } - // - // NOTE: We need an IPAddress now, the only way to create it from a - // SocketAddress is via IPEndPoint. This ought to be simpler. - // - if (pAddressInfo->ai_family == AddressFamily.InterNetwork) - { - addresses.Add(((IPEndPoint)IPEndPointStatics.Any.Create(sockaddr)).Address); - } - else - { - addresses.Add(((IPEndPoint)IPEndPointStatics.IPv6Any.Create(sockaddr)).Address); - } + addresses.Add(CreateIPv4Address(pAddressInfo->ai_addr, pAddressInfo->ai_addrlen)); + } + else if (pAddressInfo->ai_family == AddressFamily.InterNetworkV6 && SocketProtocolSupportPal.OSSupportsIPv6) + { + addresses.Add(CreateIPv6Address(pAddressInfo->ai_addr, pAddressInfo->ai_addrlen)); } // // Next addressinfo entry @@ -385,10 +383,299 @@ public static void EnsureSocketsAreInitialized() throw new SocketException((int)errorCode); } + s_getAddrInfoExSupported = TestGetAddrInfoEx(); + Volatile.Write(ref s_initialized, true); } } } } + + private static bool TestGetAddrInfoEx() + { + using (SafeLibraryHandle libHandle = Interop.Kernel32.LoadLibraryExW(Interop.Libraries.Ws2_32, IntPtr.Zero, 0)) + { + return Interop.Kernel32.GetProcAddress(libHandle, nameof(Interop.Winsock.GetAddrInfoExCancel)) != IntPtr.Zero; + } + } + + public static unsafe void GetAddrInfoAsync(string name, DnsResolveAsyncResult asyncResult) + { + try + { } + finally + { + GetAddrInfoExContext* context = GetAddrInfoExContext.AllocateContext(name, asyncResult); + + AddressInfoEx hints = new AddressInfoEx(); + hints.ai_flags = AddressInfoHints.AI_CANONNAME; + hints.ai_family = AddressFamily.Unspecified; // Gets all address families + + SocketError errorCode = + (SocketError)Interop.Winsock.GetAddrInfoExW(name, null, 0 /* NS_ALL*/, IntPtr.Zero, ref hints, out context->Result, IntPtr.Zero, ref context->Overlapped, s_getAddrInfoExCallback, out context->CancelHandle); + + if (errorCode != SocketError.IOPending) + ProcessResult(errorCode, context); + } + } + + private static unsafe void GetAddressInfoExCallback([In] int error, [In] int bytes, [In] NativeOverlapped* overlapped) + { + try + { } + finally + { + // Can be casted directly to QueryContext* because the overlapped is its first field + GetAddrInfoExContext* context = (GetAddrInfoExContext*)overlapped; + + ProcessResult((SocketError)error, context); + } + } + + private static unsafe void ProcessResult(SocketError errorCode, GetAddrInfoExContext* context) + { + GetAddrInfoExState state = context->GetQueryState(); + + if (state == null || !state.SetCallbackStartedOrCanceled()) + return; + + try + { + if (errorCode != SocketError.Success) + { + state.CompleteAsyncResult(new SocketException((int)errorCode)); + return; + } + + AddressInfoEx* result = context->Result; + string canonicalName = null; + + List addresses = new List(); + + while (result != null) + { + if (canonicalName == null && result->ai_canonname != IntPtr.Zero) + canonicalName = Marshal.PtrToStringUni(result->ai_canonname); + + IPAddress ipAddress = null; + + if (result->ai_family == AddressFamily.InterNetwork) + { + ipAddress = CreateIPv4Address(result->ai_addr, result->ai_addrlen); + } + else if (SocketProtocolSupportPal.OSSupportsIPv6 && result->ai_family == AddressFamily.InterNetworkV6) + { + ipAddress = CreateIPv6Address(result->ai_addr, result->ai_addrlen); + } + + if (ipAddress != null) + addresses.Add(ipAddress); + + result = result->ai_next; + } + + if (canonicalName == null) + canonicalName = state.HostAddress; + + state.CompleteAsyncResult(new IPHostEntry + { + HostName = canonicalName, + Aliases = Array.Empty(), + AddressList = addresses.ToArray() + }); + } + finally + { + state.Dispose(); + } + } + + private static unsafe IPAddress CreateIPv4Address(byte* socketAddress, int addressLength) + { + const int IPv4SocketAddressSize = 16; + + if (addressLength != IPv4SocketAddressSize) + return null; + + long address = (long)( + (socketAddress[4] & 0x000000FF) | + (socketAddress[5] << 8 & 0x0000FF00) | + (socketAddress[6] << 16 & 0x00FF0000) | + (socketAddress[7] << 24) + ) & 0x00000000FFFFFFFF; + + return new IPAddress(address); + } + + private static unsafe IPAddress CreateIPv6Address(byte* socketAddress, int addressLength) + { + const int IPv6SocketAddressSize = 28; + const int IPv6AddressBytes = 16; + + if (addressLength != IPv6SocketAddressSize) + return null; + + byte[] address = new byte[IPv6AddressBytes]; + for (int i = 0; i < address.Length; i++) + { + address[i] = socketAddress[i + 8]; + } + + long scope = (long)((socketAddress[27] << 24) + + (socketAddress[26] << 16) + + (socketAddress[25] << 8) + + (socketAddress[24])); + + return new IPAddress(address, scope); + } + + #region GetAddrInfoAsync Helper Classes + + private sealed unsafe class GetAddrInfoExState : CriticalFinalizerObject, IDisposable + { + private IntPtr m_context; + private int m_callbackStartedOrCanceled; + private DnsResolveAsyncResult m_asyncResult; + + public string HostAddress { get; } + + public GetAddrInfoExState(string hostAddress, DnsResolveAsyncResult asyncResult) + { + HostAddress = hostAddress; + m_asyncResult = asyncResult; + } + + [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] + public void SetQueryContext(IntPtr context) + { + m_context = context; + } + + [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] + public bool SetCallbackStartedOrCanceled() + { + return Interlocked.Exchange(ref m_callbackStartedOrCanceled, 1) == 0; + } + + public void CompleteAsyncResult(object o) + { + Task.Run(() => m_asyncResult.InvokeCallback(o)); + } + + [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] + ~GetAddrInfoExState() + { + // The finalizer should only be called when the AppDomain is unloading while there was a pending operation. + // Calling GetAddrInfoExCancel will invoke the callback inline with WSA_E_CANCELLED error code. + + // If the callback is already invoked, let the traditional Dispose clear the resources. + // This should not be possible, because the finalizer would not have been called if the object was still alive. + // This is an extra protection in case the object was somehow resurrected. + if (!SetCallbackStartedOrCanceled()) + return; + + ReleaseResources(cancelQuery: true); + } + + [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] + public void Dispose() + { + GC.SuppressFinalize(this); + ReleaseResources(cancelQuery: false); + } + + [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] + private void ReleaseResources(bool cancelQuery) + { + var ptr = Interlocked.Exchange(ref m_context, IntPtr.Zero); + + if (ptr == IntPtr.Zero) + return; + + var context = (GetAddrInfoExContext*)ptr; + + if (cancelQuery) + context->CancelQuery(); + + GetAddrInfoExContext.FreeContext(context); + } + } + + [StructLayout(LayoutKind.Sequential)] + private unsafe struct GetAddrInfoExContext + { + private static readonly int Size = Marshal.SizeOf(); + + public NativeOverlapped Overlapped; + public AddressInfoEx* Result; + public IntPtr QueryStateHandle; + public IntPtr CancelHandle; + + public GetAddrInfoExContext(GetAddrInfoExState state) + { + Overlapped = new NativeOverlapped(); + Result = null; + + var handle = GCHandle.Alloc(state, GCHandleType.Normal); + QueryStateHandle = GCHandle.ToIntPtr(handle); + + CancelHandle = IntPtr.Zero; + } + + public GetAddrInfoExState GetQueryState() + { + var stateHandle = Interlocked.Exchange(ref QueryStateHandle, IntPtr.Zero); + + if (stateHandle == IntPtr.Zero) + return null; + + var handle = GCHandle.FromIntPtr(stateHandle); + + if (!handle.IsAllocated) + return null; + + var state = (GetAddrInfoExState)handle.Target; + handle.Free(); + + return state; + } + + [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] + public void CancelQuery() + { + if (CancelHandle != IntPtr.Zero) + { + Interop.Winsock.GetAddrInfoExCancel(ref CancelHandle); + CancelHandle = IntPtr.Zero; + } + } + + public static GetAddrInfoExContext* AllocateContext(string hostAddress, DnsResolveAsyncResult asyncResult) + { + GetAddrInfoExContext* context = (GetAddrInfoExContext*)Marshal.AllocHGlobal(Size); + GetAddrInfoExState state = new GetAddrInfoExState(hostAddress, asyncResult); + + *context = new GetAddrInfoExContext(state); + state.SetQueryContext((IntPtr)context); + return context; + } + + [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] + public static void FreeContext(GetAddrInfoExContext* context) + { + if (context->Result != null) + { + Interop.Winsock.FreeAddrInfoEx(context->Result); + context->Result = null; + } + + // There is no need to free the GCHandle held in 'QueryStateHandle' + // The handle is freed by 'GetQueryState' unless the AppDomain is unloaded. + + Marshal.FreeHGlobal((IntPtr)context); + } + } + + #endregion } } diff --git a/src/System.Net.NameResolution/tests/PalTests/Fakes/FakeContextAwareResult.cs b/src/System.Net.NameResolution/tests/PalTests/Fakes/FakeContextAwareResult.cs new file mode 100644 index 000000000000..8a9615c9d2da --- /dev/null +++ b/src/System.Net.NameResolution/tests/PalTests/Fakes/FakeContextAwareResult.cs @@ -0,0 +1,91 @@ +// Licensed to the .NET Foundation under one or more agreements. +// 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.Threading; + +namespace System.Net +{ + internal partial class ContextAwareResult : IAsyncResult + { + private AsyncCallback _callback; + + private static Func _resultFactory; + + public static void FakeSetResultFactory(Func resultFactory) + { + _resultFactory = resultFactory; + } + + public object AsyncState + { + get + { + throw new NotImplementedException(); + } + } + + internal bool EndCalled + { + get; + set; + } + + internal object Result + { + get + { + return _resultFactory?.Invoke(); + } + } + + public WaitHandle AsyncWaitHandle + { + get + { + throw new NotImplementedException(); + } + } + + public bool CompletedSynchronously + { + get + { + // Simulate sync completion: + return true; + } + } + + public bool IsCompleted + { + get + { + throw new NotImplementedException(); + } + } + + internal ContextAwareResult(object myObject, object myState, AsyncCallback myCallBack) + { + _callback = myCallBack; + } + + internal object StartPostingAsyncOp(bool lockCapture) + { + return null; + } + + internal bool FinishPostingAsyncOp() + { + return true; + } + + internal void InvokeCallback(object result) + { + _callback.Invoke(this); + } + + internal void InternalWaitForCompletion() { } + + + } +} diff --git a/src/System.Net.NameResolution/tests/PalTests/System.Net.NameResolution.Pal.Tests.csproj b/src/System.Net.NameResolution/tests/PalTests/System.Net.NameResolution.Pal.Tests.csproj index 333aa11cc124..ef0a35515ffe 100644 --- a/src/System.Net.NameResolution/tests/PalTests/System.Net.NameResolution.Pal.Tests.csproj +++ b/src/System.Net.NameResolution/tests/PalTests/System.Net.NameResolution.Pal.Tests.csproj @@ -24,6 +24,7 @@ + @@ -34,6 +35,9 @@ ProductionCode\System\Net\NameResolutionUtilities.cs + + ProductionCode\System\Net\DnsResolveAsyncResult.cs + Common\System\Net\Sockets\ProtocolType.cs @@ -116,6 +120,24 @@ Interop\Windows\Winsock\SafeFreeAddrInfo.cs + + Interop\Windows\Winsock\AddressInfoEx.cs + + + Interop\Windows\Winsock\Interop.GetAddrInfoExW.cs + + + Common\Microsoft\Win32\SafeHandles\SafeLibraryHandle.cs + + + Interop\Windows\Kernel32\Interop.GetProcAddress.cs + + + Interop\Windows\Kernel32\Interop.LoadLibraryEx.cs + + + Interop\Windows\Kernel32\Interop.FreeLibrary.cs + @@ -171,4 +193,4 @@ - + \ No newline at end of file diff --git a/src/System.Net.NameResolution/tests/UnitTests/Fakes/FakeNameResolutionPal.cs b/src/System.Net.NameResolution/tests/UnitTests/Fakes/FakeNameResolutionPal.cs index 74da893f328e..a3b44a097889 100644 --- a/src/System.Net.NameResolution/tests/UnitTests/Fakes/FakeNameResolutionPal.cs +++ b/src/System.Net.NameResolution/tests/UnitTests/Fakes/FakeNameResolutionPal.cs @@ -10,6 +10,8 @@ namespace System.Net { internal static class NameResolutionPal { + public static bool SupportsGetAddrInfoAsync => false; + internal static int FakesEnsureSocketsAreInitializedCallCount { get; @@ -49,6 +51,11 @@ internal static string TryGetNameInfo(IPAddress address, out SocketError errorCo throw new NotImplementedException(); } + internal static void GetAddrInfoAsync(string name, DnsResolveAsyncResult asyncResult) + { + throw new NotImplementedException(); + } + internal static IPHostEntry GetHostByAddr(IPAddress address) { throw new NotImplementedException(); diff --git a/src/System.Net.NameResolution/tests/UnitTests/System.Net.NameResolution.Unit.Tests.csproj b/src/System.Net.NameResolution/tests/UnitTests/System.Net.NameResolution.Unit.Tests.csproj index e99d16ebed38..b52ff2aafce0 100644 --- a/src/System.Net.NameResolution/tests/UnitTests/System.Net.NameResolution.Unit.Tests.csproj +++ b/src/System.Net.NameResolution/tests/UnitTests/System.Net.NameResolution.Unit.Tests.csproj @@ -26,6 +26,9 @@ ProductionCode\System\Net\DNS.cs + + + ProductionCode\System\Net\DnsResolveAsyncResult.cs From 77ccb1c1011f54b1a9b884b4fda36491c84add26 Mon Sep 17 00:00:00 2001 From: JeffCyr Date: Tue, 6 Feb 2018 11:58:00 -0500 Subject: [PATCH 02/10] Applied code review recommendations --- .../Interop/Windows/Winsock/AddressInfoEx.cs | 4 + .../Windows/Winsock/Interop.GetAddrInfoExW.cs | 35 ++-- .../src/System/Net/SocketAddressPal.Unix.cs | 16 +- .../System/Net/SocketAddressPal.Windows.cs | 12 +- .../src/System.Net.NameResolution.csproj | 6 + .../src/System/Net/DNS.cs | 40 +++-- .../src/System/Net/DnsResolveAsyncResult.cs | 32 ++-- .../src/System/Net/NameResolutionPal.Unix.cs | 2 +- .../System/Net/NameResolutionPal.Windows.cs | 157 +++++++++--------- ...System.Net.NameResolution.Pal.Tests.csproj | 6 + 10 files changed, 178 insertions(+), 132 deletions(-) diff --git a/src/Common/src/Interop/Windows/Winsock/AddressInfoEx.cs b/src/Common/src/Interop/Windows/Winsock/AddressInfoEx.cs index a50512058afa..0972101831a6 100644 --- a/src/Common/src/Interop/Windows/Winsock/AddressInfoEx.cs +++ b/src/Common/src/Interop/Windows/Winsock/AddressInfoEx.cs @@ -1,3 +1,7 @@ +// Licensed to the .NET Foundation under one or more agreements. +// 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.Net.Internals; using System.Runtime.InteropServices; diff --git a/src/Common/src/Interop/Windows/Winsock/Interop.GetAddrInfoExW.cs b/src/Common/src/Interop/Windows/Winsock/Interop.GetAddrInfoExW.cs index 29a137685825..902799d0d2ff 100644 --- a/src/Common/src/Interop/Windows/Winsock/Interop.GetAddrInfoExW.cs +++ b/src/Common/src/Interop/Windows/Winsock/Interop.GetAddrInfoExW.cs @@ -1,6 +1,9 @@ -using System; +// Licensed to the .NET Foundation under one or more agreements. +// 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; using System.Net.Sockets; -using System.Runtime.ConstrainedExecution; using System.Runtime.InteropServices; using System.Threading; @@ -8,29 +11,27 @@ internal static partial class Interop { internal static partial class Winsock { - internal unsafe delegate void GetAddrInfoExCompletionCallback([In] int error, [In] int bytes, [In] NativeOverlapped* overlapped); + internal unsafe delegate void LPLOOKUPSERVICE_COMPLETION_ROUTINE([In] int dwError, [In] int dwBytes, [In] NativeOverlapped* lpOverlapped); - [DllImport(Interop.Libraries.Ws2_32, ExactSpelling = true, CharSet = CharSet.Unicode, BestFitMapping = false, ThrowOnUnmappableChar = true, SetLastError = true)] + [DllImport(Interop.Libraries.Ws2_32, ExactSpelling = true, CharSet = CharSet.Unicode, SetLastError = true)] internal static extern unsafe int GetAddrInfoExW( - [In] string name, - [In] string serviceName, - [In] int namespaceId, - [In] IntPtr providerId, - [In] ref AddressInfoEx hints, - [Out] out AddressInfoEx* result, + [In] string pName, + [In] string pServiceName, + [In] int dwNamespace, + [In] IntPtr lpNspId, + [In] ref AddressInfoEx pHints, + [Out] out AddressInfoEx* ppResult, [In] IntPtr timeout, - [In] ref NativeOverlapped overlapped, - [In] GetAddrInfoExCompletionCallback callback, - [Out] out IntPtr cancelHandle + [In] ref NativeOverlapped lpOverlapped, + [In] LPLOOKUPSERVICE_COMPLETION_ROUTINE lpCompletionRoutine, + [Out] out IntPtr lpNameHandle ); - [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] [DllImport("ws2_32.dll", ExactSpelling = true, SetLastError = true)] - internal static extern unsafe void FreeAddrInfoEx([In] AddressInfoEx* addressInfo); + internal static extern unsafe void FreeAddrInfoEx([In] AddressInfoEx* pAddrInfo); - [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] [DllImport("ws2_32.dll", ExactSpelling = true, SetLastError = true)] - internal static extern int GetAddrInfoExCancel([In] ref IntPtr cancelHandle); + internal static extern int GetAddrInfoExCancel([In] ref IntPtr lpHandle); } } diff --git a/src/Common/src/System/Net/SocketAddressPal.Unix.cs b/src/Common/src/System/Net/SocketAddressPal.Unix.cs index f7eaf04fbde2..082a8babbc3a 100644 --- a/src/Common/src/System/Net/SocketAddressPal.Unix.cs +++ b/src/Common/src/System/Net/SocketAddressPal.Unix.cs @@ -103,10 +103,15 @@ public static unsafe void SetPort(byte[] buffer, ushort port) } public static unsafe uint GetIPv4Address(byte[] buffer) + { + return GetIPv4Address(new ReadOnlySpan(buffer)); + } + + public static unsafe uint GetIPv4Address(ReadOnlySpan buffer) { uint ipAddress; Interop.Error err; - fixed (byte* rawAddress = buffer) + fixed (byte* rawAddress = &MemoryMarshal.GetReference(buffer)) { err = Interop.Sys.GetIPv4Address(rawAddress, buffer.Length, &ipAddress); } @@ -115,11 +120,16 @@ public static unsafe uint GetIPv4Address(byte[] buffer) return ipAddress; } - public static unsafe void GetIPv6Address(byte[] buffer, Span address, out uint scope) + public static void GetIPv6Address(byte[] buffer, Span address, out uint scope) + { + GetIPv6Address(new ReadOnlySpan(buffer), address, out scope); + } + + public static unsafe void GetIPv6Address(ReadOnlySpan buffer, Span address, out uint scope) { uint localScope; Interop.Error err; - fixed (byte* rawAddress = buffer) + fixed (byte* rawAddress = &MemoryMarshal.GetReference(buffer)) fixed (byte* ipAddress = &MemoryMarshal.GetReference(address)) { err = Interop.Sys.GetIPv6Address(rawAddress, buffer.Length, ipAddress, address.Length, &localScope); diff --git a/src/Common/src/System/Net/SocketAddressPal.Windows.cs b/src/Common/src/System/Net/SocketAddressPal.Windows.cs index 32685d48826b..1b66d505eed5 100644 --- a/src/Common/src/System/Net/SocketAddressPal.Windows.cs +++ b/src/Common/src/System/Net/SocketAddressPal.Windows.cs @@ -39,6 +39,11 @@ public static unsafe void SetPort(byte[] buffer, ushort port) } public static unsafe uint GetIPv4Address(byte[] buffer) + { + return GetIPv4Address(new ReadOnlySpan(buffer)); + } + + public static unsafe uint GetIPv4Address(ReadOnlySpan buffer) { unchecked { @@ -49,7 +54,12 @@ public static unsafe uint GetIPv4Address(byte[] buffer) } } - public static unsafe void GetIPv6Address(byte[] buffer, Span address, out uint scope) + public static void GetIPv6Address(byte[] buffer, Span address, out uint scope) + { + GetIPv6Address(new ReadOnlySpan(buffer), address, out scope); + } + + public static unsafe void GetIPv6Address(ReadOnlySpan buffer, Span address, out uint scope) { for (int i = 0; i < address.Length; i++) { diff --git a/src/System.Net.NameResolution/src/System.Net.NameResolution.csproj b/src/System.Net.NameResolution/src/System.Net.NameResolution.csproj index 54c6db45fcf9..5949234f98d6 100644 --- a/src/System.Net.NameResolution/src/System.Net.NameResolution.csproj +++ b/src/System.Net.NameResolution/src/System.Net.NameResolution.csproj @@ -45,6 +45,9 @@ Common\System\Net\IPEndPointStatics.cs + + + Common\System\Net\ByteOrder.cs @@ -72,6 +75,9 @@ Common\System\Net\SocketProtocolSupportPal.Windows + + + Common\System\Net\SocketAddressPal.Windows diff --git a/src/System.Net.NameResolution/src/System/Net/DNS.cs b/src/System.Net.NameResolution/src/System/Net/DNS.cs index 00c07be09af1..a68140bdf73e 100644 --- a/src/System.Net.NameResolution/src/System/Net/DNS.cs +++ b/src/System.Net.NameResolution/src/System/Net/DNS.cs @@ -41,17 +41,22 @@ public static IPHostEntry GetHostByName(string hostName) return InternalGetHostByName(hostName, false); } - private static IPHostEntry InternalGetHostByName(string hostName, bool includeIPv6) + private static void ValidateHostName(string hostName) { - if (NetEventSource.IsEnabled) NetEventSource.Enter(null, hostName); - IPHostEntry ipHostEntry = null; - if (hostName.Length > MaxHostName // If 255 chars, the last one must be a dot. || hostName.Length == MaxHostName && hostName[MaxHostName - 1] != '.') { throw new ArgumentOutOfRangeException(nameof(hostName), SR.Format(SR.net_toolong, nameof(hostName), MaxHostName.ToString(NumberFormatInfo.CurrentInfo))); } + } + + private static IPHostEntry InternalGetHostByName(string hostName, bool includeIPv6) + { + if (NetEventSource.IsEnabled) NetEventSource.Enter(null, hostName); + IPHostEntry ipHostEntry = null; + + ValidateHostName(hostName); // // IPv6 Changes: IPv6 requires the use of getaddrinfo() rather @@ -258,13 +263,13 @@ private static void ResolveCallback(object context) IPHostEntry hostEntry; try { - if (result.address != null) + if (result.IpAddress != null) { - hostEntry = InternalGetHostByAddress(result.address, result.includeIPv6); + hostEntry = InternalGetHostByAddress(result.IpAddress, result.IncludeIPv6); } else { - hostEntry = InternalGetHostByName(result.hostName, result.includeIPv6); + hostEntry = InternalGetHostByName(result.HostName, result.IncludeIPv6); } } catch (OutOfMemoryException) @@ -292,20 +297,20 @@ private static IAsyncResult HostResolutionBeginHelper(string hostName, bool just if (NetEventSource.IsEnabled) NetEventSource.Info(null, hostName); // See if it's an IP Address. - IPAddress address; + IPAddress ipAddress; DnsResolveAsyncResult asyncResult; - if (IPAddress.TryParse(hostName, out address)) + if (IPAddress.TryParse(hostName, out ipAddress)) { - if (throwOnIIPAny && (address.Equals(IPAddress.Any) || address.Equals(IPAddress.IPv6Any))) + if (throwOnIIPAny && (ipAddress.Equals(IPAddress.Any) || ipAddress.Equals(IPAddress.IPv6Any))) { throw new ArgumentException(SR.net_invalid_ip_addr, nameof(hostName)); } - asyncResult = new DnsResolveAsyncResult(address, null, includeIPv6, state, requestCallback); + asyncResult = new DnsResolveAsyncResult(ipAddress, null, includeIPv6, state, requestCallback); if (justReturnParsedIp) { - IPHostEntry hostEntry = NameResolutionUtilities.GetUnresolvedAnswer(address); + IPHostEntry hostEntry = NameResolutionUtilities.GetUnresolvedAnswer(ipAddress); asyncResult.StartPostingAsyncOp(false); asyncResult.InvokeCallback(hostEntry); asyncResult.FinishPostingAsyncOp(); @@ -320,8 +325,15 @@ private static IAsyncResult HostResolutionBeginHelper(string hostName, bool just // Set up the context, possibly flow. asyncResult.StartPostingAsyncOp(false); - if (NameResolutionPal.SupportsGetAddrInfoAsync && includeIPv6 && SocketProtocolSupportPal.OSSupportsIPv6 && address == null) + // GetHostByName is used instead of GetAddrInfo if ipv6 is not supported. + // See the comment in InternalGetHostByName for further explanation. + bool useGetHostByName = includeIPv6 || SocketProtocolSupportPal.OSSupportsIPv6; + + // If the OS supports it and 'hostName' is not an IP Address, resolve the name asynchronously + // instead of calling the synchronous version in the ThreadPool. + if (NameResolutionPal.SupportsGetAddrInfoAsync && ipAddress == null && !useGetHostByName) { + ValidateHostName(hostName); NameResolutionPal.GetAddrInfoAsync(hostName, asyncResult); } else @@ -595,7 +607,7 @@ public static IPHostEntry EndResolve(IAsyncResult asyncResult) } catch (SocketException ex) { - IPAddress address = ((DnsResolveAsyncResult)asyncResult).address; + IPAddress address = ((DnsResolveAsyncResult)asyncResult).IpAddress; if (address == null) throw; // BeginResolve was called with a HostName, not an IPAddress diff --git a/src/System.Net.NameResolution/src/System/Net/DnsResolveAsyncResult.cs b/src/System.Net.NameResolution/src/System/Net/DnsResolveAsyncResult.cs index 56dd163034dc..ca4f700af124 100644 --- a/src/System.Net.NameResolution/src/System/Net/DnsResolveAsyncResult.cs +++ b/src/System.Net.NameResolution/src/System/Net/DnsResolveAsyncResult.cs @@ -1,25 +1,29 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + namespace System.Net { - internal class DnsResolveAsyncResult : ContextAwareResult + internal sealed class DnsResolveAsyncResult : ContextAwareResult { + internal string HostName { get; } + internal bool IncludeIPv6 { get; } + internal IPAddress IpAddress { get; } + // Forward lookup - internal DnsResolveAsyncResult(string hostName, object myObject, bool includeIPv6, object myState, AsyncCallback myCallBack) : - base(myObject, myState, myCallBack) + internal DnsResolveAsyncResult(string hostName, object myObject, bool includeIPv6, object myState, AsyncCallback myCallBack) + : base(myObject, myState, myCallBack) { - this.hostName = hostName; - this.includeIPv6 = includeIPv6; + HostName = hostName; + IncludeIPv6 = includeIPv6; } // Reverse lookup - internal DnsResolveAsyncResult(IPAddress address, object myObject, bool includeIPv6, object myState, AsyncCallback myCallBack) : - base(myObject, myState, myCallBack) + internal DnsResolveAsyncResult(IPAddress ipAddress, object myObject, bool includeIPv6, object myState, AsyncCallback myCallBack) + : base(myObject, myState, myCallBack) { - this.includeIPv6 = includeIPv6; - this.address = address; + IncludeIPv6 = includeIPv6; + IpAddress = ipAddress; } - - internal readonly string hostName; - internal bool includeIPv6; - internal IPAddress address; } -} \ No newline at end of file +} diff --git a/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Unix.cs b/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Unix.cs index 8ad4c869dc27..c552d0e35f60 100644 --- a/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Unix.cs +++ b/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Unix.cs @@ -13,7 +13,7 @@ namespace System.Net { internal static partial class NameResolutionPal { - public static bool SupportsGetAddrInfoAsync => false; + public const bool SupportsGetAddrInfoAsync = false; private static SocketError GetSocketErrorForErrno(int errno) { diff --git a/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Windows.cs b/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Windows.cs index 2f05b3ba8a6f..752a8fee2c03 100644 --- a/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Windows.cs +++ b/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Windows.cs @@ -4,7 +4,6 @@ using System.Collections.Generic; using System.Net.Sockets; -using System.Runtime.ConstrainedExecution; using System.Runtime.InteropServices; using System.Text; using System.Threading; @@ -20,10 +19,11 @@ internal static class NameResolutionPal // used by GetHostName() to preallocate a buffer for the call to gethostname. // private const int HostNameBufferLength = 256; + private static bool s_initialized; private static readonly object s_initializedLock = new object(); - private static readonly unsafe Interop.Winsock.GetAddrInfoExCompletionCallback s_getAddrInfoExCallback = GetAddressInfoExCallback; + private static readonly unsafe Interop.Winsock.LPLOOKUPSERVICE_COMPLETION_ROUTINE s_getAddrInfoExCallback = GetAddressInfoExCallback; private static bool s_getAddrInfoExSupported; public static bool SupportsGetAddrInfoAsync @@ -261,13 +261,17 @@ public static unsafe SocketError TryGetAddrInfo(string name, out IPHostEntry hos // We also filter based on whether IPv6 is supported on the current // platform / machine. // + var socketAddress = new ReadOnlySpan(pAddressInfo->ai_addr, pAddressInfo->ai_addrlen); + if (pAddressInfo->ai_family == AddressFamily.InterNetwork) { - addresses.Add(CreateIPv4Address(pAddressInfo->ai_addr, pAddressInfo->ai_addrlen)); + if (socketAddress.Length == SocketAddressPal.IPv4AddressSize) + addresses.Add(CreateIPv4Address(socketAddress)); } else if (pAddressInfo->ai_family == AddressFamily.InterNetworkV6 && SocketProtocolSupportPal.OSSupportsIPv6) { - addresses.Add(CreateIPv6Address(pAddressInfo->ai_addr, pAddressInfo->ai_addrlen)); + if (socketAddress.Length == SocketAddressPal.IPv6AddressSize) + addresses.Add(CreateIPv6Address(socketAddress)); } // // Next addressinfo entry @@ -383,7 +387,7 @@ public static void EnsureSocketsAreInitialized() throw new SocketException((int)errorCode); } - s_getAddrInfoExSupported = TestGetAddrInfoEx(); + s_getAddrInfoExSupported = GetAddrInfoExSupportsOverlapped(); Volatile.Write(ref s_initialized, true); } @@ -391,51 +395,50 @@ public static void EnsureSocketsAreInitialized() } } - private static bool TestGetAddrInfoEx() + private static bool GetAddrInfoExSupportsOverlapped() { using (SafeLibraryHandle libHandle = Interop.Kernel32.LoadLibraryExW(Interop.Libraries.Ws2_32, IntPtr.Zero, 0)) { + if (libHandle.IsInvalid) + return false; + + // We can't just check that 'GetAddrInfoEx' exists, because it existed before supporting overlapped. + // The existance of 'GetAddrInfoExCancel' indicates that overlapped is supported. return Interop.Kernel32.GetProcAddress(libHandle, nameof(Interop.Winsock.GetAddrInfoExCancel)) != IntPtr.Zero; } } public static unsafe void GetAddrInfoAsync(string name, DnsResolveAsyncResult asyncResult) { - try - { } - finally - { - GetAddrInfoExContext* context = GetAddrInfoExContext.AllocateContext(name, asyncResult); + GetAddrInfoExContext* context = GetAddrInfoExContext.AllocateContext(name, asyncResult); - AddressInfoEx hints = new AddressInfoEx(); - hints.ai_flags = AddressInfoHints.AI_CANONNAME; - hints.ai_family = AddressFamily.Unspecified; // Gets all address families + AddressInfoEx hints = new AddressInfoEx(); + hints.ai_flags = AddressInfoHints.AI_CANONNAME; + hints.ai_family = AddressFamily.Unspecified; // Gets all address families - SocketError errorCode = - (SocketError)Interop.Winsock.GetAddrInfoExW(name, null, 0 /* NS_ALL*/, IntPtr.Zero, ref hints, out context->Result, IntPtr.Zero, ref context->Overlapped, s_getAddrInfoExCallback, out context->CancelHandle); + SocketError errorCode = + (SocketError)Interop.Winsock.GetAddrInfoExW(name, null, 0 /* NS_ALL*/, IntPtr.Zero, ref hints, out context->Result, IntPtr.Zero, ref context->Overlapped, s_getAddrInfoExCallback, out context->CancelHandle); - if (errorCode != SocketError.IOPending) - ProcessResult(errorCode, context); - } + if (errorCode != SocketError.IOPending) + ProcessResult(errorCode, context); } private static unsafe void GetAddressInfoExCallback([In] int error, [In] int bytes, [In] NativeOverlapped* overlapped) { - try - { } - finally - { - // Can be casted directly to QueryContext* because the overlapped is its first field - GetAddrInfoExContext* context = (GetAddrInfoExContext*)overlapped; + // Can be casted directly to GetAddrInfoExContext* because the overlapped is its first field + GetAddrInfoExContext* context = (GetAddrInfoExContext*)overlapped; - ProcessResult((SocketError)error, context); - } + ProcessResult((SocketError)error, context); } private static unsafe void ProcessResult(SocketError errorCode, GetAddrInfoExContext* context) { GetAddrInfoExState state = context->GetQueryState(); + // 'state' may be null or 'SetCallbackStartedOrCanceled' may return false if + // the AppDomain started unloading before the callback was raised. + // In this case we don't need to free the context, it will be done in + // GetAddrInfoExState's finalizer. if (state == null || !state.SetCallbackStartedOrCanceled()) return; @@ -458,14 +461,17 @@ private static unsafe void ProcessResult(SocketError errorCode, GetAddrInfoExCon canonicalName = Marshal.PtrToStringUni(result->ai_canonname); IPAddress ipAddress = null; + var socketAddress = new ReadOnlySpan(result->ai_addr, result->ai_addrlen); if (result->ai_family == AddressFamily.InterNetwork) { - ipAddress = CreateIPv4Address(result->ai_addr, result->ai_addrlen); + if (socketAddress.Length == SocketAddressPal.IPv4AddressSize) + ipAddress = CreateIPv4Address(socketAddress); } else if (SocketProtocolSupportPal.OSSupportsIPv6 && result->ai_family == AddressFamily.InterNetworkV6) { - ipAddress = CreateIPv6Address(result->ai_addr, result->ai_addrlen); + if (socketAddress.Length == SocketAddressPal.IPv6AddressSize) + ipAddress = CreateIPv6Address(socketAddress); } if (ipAddress != null) @@ -490,79 +496,60 @@ private static unsafe void ProcessResult(SocketError errorCode, GetAddrInfoExCon } } - private static unsafe IPAddress CreateIPv4Address(byte* socketAddress, int addressLength) + private static unsafe IPAddress CreateIPv4Address(ReadOnlySpan socketAddress) { - const int IPv4SocketAddressSize = 16; - - if (addressLength != IPv4SocketAddressSize) - return null; - - long address = (long)( - (socketAddress[4] & 0x000000FF) | - (socketAddress[5] << 8 & 0x0000FF00) | - (socketAddress[6] << 16 & 0x00FF0000) | - (socketAddress[7] << 24) - ) & 0x00000000FFFFFFFF; - + long address = (long)SocketAddressPal.GetIPv4Address(socketAddress) & 0x0FFFFFFFF; return new IPAddress(address); } - private static unsafe IPAddress CreateIPv6Address(byte* socketAddress, int addressLength) + private static unsafe IPAddress CreateIPv6Address(ReadOnlySpan socketAddress) { - const int IPv6SocketAddressSize = 28; - const int IPv6AddressBytes = 16; - - if (addressLength != IPv6SocketAddressSize) - return null; - - byte[] address = new byte[IPv6AddressBytes]; - for (int i = 0; i < address.Length; i++) - { - address[i] = socketAddress[i + 8]; - } + Span address = stackalloc byte[IPAddressParserStatics.IPv6AddressBytes]; + uint scope; + SocketAddressPal.GetIPv6Address(socketAddress, address, out scope); - long scope = (long)((socketAddress[27] << 24) + - (socketAddress[26] << 16) + - (socketAddress[25] << 8) + - (socketAddress[24])); - - return new IPAddress(address, scope); + return new IPAddress(address, (long)scope); } #region GetAddrInfoAsync Helper Classes - private sealed unsafe class GetAddrInfoExState : CriticalFinalizerObject, IDisposable + private sealed unsafe class GetAddrInfoExState : IDisposable { - private IntPtr m_context; - private int m_callbackStartedOrCanceled; - private DnsResolveAsyncResult m_asyncResult; + private IntPtr _context; + private int _callbackStartedOrCanceled; + private DnsResolveAsyncResult _asyncResult; public string HostAddress { get; } public GetAddrInfoExState(string hostAddress, DnsResolveAsyncResult asyncResult) { HostAddress = hostAddress; - m_asyncResult = asyncResult; + _asyncResult = asyncResult; } - [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] public void SetQueryContext(IntPtr context) { - m_context = context; + _context = context; } - [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] public bool SetCallbackStartedOrCanceled() { - return Interlocked.Exchange(ref m_callbackStartedOrCanceled, 1) == 0; + return Interlocked.Exchange(ref _callbackStartedOrCanceled, 1) == 0; } public void CompleteAsyncResult(object o) { - Task.Run(() => m_asyncResult.InvokeCallback(o)); + // We don't want to expose the GetAddrInfoEx callback thread to user code. + + var state = Tuple.Create(_asyncResult, o); + + Task.Factory.StartNew(s => + { + var t = (Tuple)s; + t.Item1.InvokeCallback(t.Item2); + }, state, CancellationToken.None, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default); } - [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] ~GetAddrInfoExState() { // The finalizer should only be called when the AppDomain is unloading while there was a pending operation. @@ -577,17 +564,15 @@ public void CompleteAsyncResult(object o) ReleaseResources(cancelQuery: true); } - [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] public void Dispose() { GC.SuppressFinalize(this); ReleaseResources(cancelQuery: false); } - [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] private void ReleaseResources(bool cancelQuery) { - var ptr = Interlocked.Exchange(ref m_context, IntPtr.Zero); + IntPtr ptr = Interlocked.Exchange(ref _context, IntPtr.Zero); if (ptr == IntPtr.Zero) return; @@ -616,7 +601,7 @@ public GetAddrInfoExContext(GetAddrInfoExState state) Overlapped = new NativeOverlapped(); Result = null; - var handle = GCHandle.Alloc(state, GCHandleType.Normal); + GCHandle handle = GCHandle.Alloc(state, GCHandleType.Normal); QueryStateHandle = GCHandle.ToIntPtr(handle); CancelHandle = IntPtr.Zero; @@ -624,12 +609,12 @@ public GetAddrInfoExContext(GetAddrInfoExState state) public GetAddrInfoExState GetQueryState() { - var stateHandle = Interlocked.Exchange(ref QueryStateHandle, IntPtr.Zero); + IntPtr stateHandle = Interlocked.Exchange(ref QueryStateHandle, IntPtr.Zero); if (stateHandle == IntPtr.Zero) return null; - var handle = GCHandle.FromIntPtr(stateHandle); + GCHandle handle = GCHandle.FromIntPtr(stateHandle); if (!handle.IsAllocated) return null; @@ -640,7 +625,6 @@ public GetAddrInfoExState GetQueryState() return state; } - [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] public void CancelQuery() { if (CancelHandle != IntPtr.Zero) @@ -653,14 +637,23 @@ public void CancelQuery() public static GetAddrInfoExContext* AllocateContext(string hostAddress, DnsResolveAsyncResult asyncResult) { GetAddrInfoExContext* context = (GetAddrInfoExContext*)Marshal.AllocHGlobal(Size); - GetAddrInfoExState state = new GetAddrInfoExState(hostAddress, asyncResult); - *context = new GetAddrInfoExContext(state); - state.SetQueryContext((IntPtr)context); + try + { + GetAddrInfoExState state = new GetAddrInfoExState(hostAddress, asyncResult); + + *context = new GetAddrInfoExContext(state); + state.SetQueryContext((IntPtr)context); + } + catch (OutOfMemoryException) + { + Marshal.FreeHGlobal((IntPtr)context); + throw; + } + return context; } - [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] public static void FreeContext(GetAddrInfoExContext* context) { if (context->Result != null) diff --git a/src/System.Net.NameResolution/tests/PalTests/System.Net.NameResolution.Pal.Tests.csproj b/src/System.Net.NameResolution/tests/PalTests/System.Net.NameResolution.Pal.Tests.csproj index ef0a35515ffe..efd79cad3bff 100644 --- a/src/System.Net.NameResolution/tests/PalTests/System.Net.NameResolution.Pal.Tests.csproj +++ b/src/System.Net.NameResolution/tests/PalTests/System.Net.NameResolution.Pal.Tests.csproj @@ -55,6 +55,9 @@ Common\System\Net\Configuration.Http.cs + + + Common\System\Net\ByteOrder.cs @@ -69,6 +72,9 @@ System\Net\SocketProtocolSupportPal.Windows + + + Common\System\Net\SocketAddressPal.Windows From a6cff67571299cb9730b29cea04ee3d83168de6a Mon Sep 17 00:00:00 2001 From: JeffCyr Date: Tue, 6 Feb 2018 12:37:37 -0500 Subject: [PATCH 03/10] Unix build fix --- .../src/System.Net.NameResolution.csproj | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/System.Net.NameResolution/src/System.Net.NameResolution.csproj b/src/System.Net.NameResolution/src/System.Net.NameResolution.csproj index 5949234f98d6..f6887340b60b 100644 --- a/src/System.Net.NameResolution/src/System.Net.NameResolution.csproj +++ b/src/System.Net.NameResolution/src/System.Net.NameResolution.csproj @@ -46,7 +46,7 @@ Common\System\Net\IPEndPointStatics.cs - + Common\System\Net\ByteOrder.cs @@ -76,7 +76,7 @@ Common\System\Net\SocketProtocolSupportPal.Windows - + Common\System\Net\SocketAddressPal.Windows @@ -146,9 +146,6 @@ - - Common\System\Net\Internals\ByteOrder.cs - Common\System\Net\ContextAwareResult.Unix.cs From 898719f23986dba43709806d34bbbb13409cf7ac Mon Sep 17 00:00:00 2001 From: JeffCyr Date: Tue, 6 Feb 2018 16:56:49 -0500 Subject: [PATCH 04/10] Unix build fix #2 --- .../src/System/Net/NameResolutionPal.Unix.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Unix.cs b/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Unix.cs index c552d0e35f60..46f70805f072 100644 --- a/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Unix.cs +++ b/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Unix.cs @@ -196,7 +196,7 @@ public static unsafe SocketError TryGetAddrInfo(string name, out IPHostEntry hos return SocketError.Success; } - internal static void GetAddrInfoAsync(string name, Dns.ResolveAsyncResult asyncResult) + internal static void GetAddrInfoAsync(string name, DnsResolveAsyncResult asyncResult) { throw new NotSupportedException(); } From c173a88b0b1a225a3bd91d80a5a3b089037158e5 Mon Sep 17 00:00:00 2001 From: JeffCyr Date: Wed, 7 Feb 2018 08:18:36 -0500 Subject: [PATCH 05/10] Unix build fix #3 --- .../tests/PalTests/System.Net.NameResolution.Pal.Tests.csproj | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/System.Net.NameResolution/tests/PalTests/System.Net.NameResolution.Pal.Tests.csproj b/src/System.Net.NameResolution/tests/PalTests/System.Net.NameResolution.Pal.Tests.csproj index efd79cad3bff..e01b311648b6 100644 --- a/src/System.Net.NameResolution/tests/PalTests/System.Net.NameResolution.Pal.Tests.csproj +++ b/src/System.Net.NameResolution/tests/PalTests/System.Net.NameResolution.Pal.Tests.csproj @@ -152,9 +152,6 @@ Common\System\Net\Internals\Interop.CheckedAccess.cs - - Common\System\Net\Internals\ByteOrder.cs - Common\System\Net\InteropIPAddressExtensions.Unix.cs From a375f91f467db78672485fbf635244388d1d1bc9 Mon Sep 17 00:00:00 2001 From: JeffCyr Date: Wed, 7 Feb 2018 16:33:03 -0500 Subject: [PATCH 06/10] NETFX build fix --- .../tests/PalTests/Configurations.props | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/System.Net.NameResolution/tests/PalTests/Configurations.props b/src/System.Net.NameResolution/tests/PalTests/Configurations.props index eddfd3a9acc5..1040c9ba37f0 100644 --- a/src/System.Net.NameResolution/tests/PalTests/Configurations.props +++ b/src/System.Net.NameResolution/tests/PalTests/Configurations.props @@ -2,8 +2,6 @@ - netstandard-Windows_NT; - netstandard-Unix; netcoreapp-Windows_NT; netcoreapp-Unix; From 795e5be1e0e70626282746e9483d6ca86b422d1b Mon Sep 17 00:00:00 2001 From: JeffCyr Date: Wed, 7 Feb 2018 21:35:10 -0500 Subject: [PATCH 07/10] Fixed useGetHostByName logic --- src/System.Net.NameResolution/src/System/Net/DNS.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Net.NameResolution/src/System/Net/DNS.cs b/src/System.Net.NameResolution/src/System/Net/DNS.cs index a68140bdf73e..d84edd635a59 100644 --- a/src/System.Net.NameResolution/src/System/Net/DNS.cs +++ b/src/System.Net.NameResolution/src/System/Net/DNS.cs @@ -327,7 +327,7 @@ private static IAsyncResult HostResolutionBeginHelper(string hostName, bool just // GetHostByName is used instead of GetAddrInfo if ipv6 is not supported. // See the comment in InternalGetHostByName for further explanation. - bool useGetHostByName = includeIPv6 || SocketProtocolSupportPal.OSSupportsIPv6; + bool useGetHostByName = !includeIPv6 && !SocketProtocolSupportPal.OSSupportsIPv6; // If the OS supports it and 'hostName' is not an IP Address, resolve the name asynchronously // instead of calling the synchronous version in the ThreadPool. From a9a3403a3e57dd77daed800da8d87f16321349ad Mon Sep 17 00:00:00 2001 From: JeffCyr Date: Thu, 8 Feb 2018 10:58:37 -0500 Subject: [PATCH 08/10] Simplified ProcessResult code --- .../src/System/Net/NameResolutionPal.Windows.cs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Windows.cs b/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Windows.cs index 752a8fee2c03..96ab2cbc232a 100644 --- a/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Windows.cs +++ b/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Windows.cs @@ -460,23 +460,19 @@ private static unsafe void ProcessResult(SocketError errorCode, GetAddrInfoExCon if (canonicalName == null && result->ai_canonname != IntPtr.Zero) canonicalName = Marshal.PtrToStringUni(result->ai_canonname); - IPAddress ipAddress = null; var socketAddress = new ReadOnlySpan(result->ai_addr, result->ai_addrlen); if (result->ai_family == AddressFamily.InterNetwork) { if (socketAddress.Length == SocketAddressPal.IPv4AddressSize) - ipAddress = CreateIPv4Address(socketAddress); + addresses.Add(CreateIPv4Address(socketAddress)); } else if (SocketProtocolSupportPal.OSSupportsIPv6 && result->ai_family == AddressFamily.InterNetworkV6) { if (socketAddress.Length == SocketAddressPal.IPv6AddressSize) - ipAddress = CreateIPv6Address(socketAddress); + addresses.Add(CreateIPv6Address(socketAddress)); } - if (ipAddress != null) - addresses.Add(ipAddress); - result = result->ai_next; } From 91864e586452f04259a1b36bcbbeba701ea89613 Mon Sep 17 00:00:00 2001 From: JeffCyr Date: Sat, 10 Feb 2018 07:50:56 -0500 Subject: [PATCH 09/10] Cleaned up cancel code --- .../Windows/kernel32/Interop.LoadLibraryEx.cs | 1 + .../src/System/Net/DNS.cs | 8 +- .../src/System/Net/NameResolutionPal.Unix.cs | 2 +- .../System/Net/NameResolutionPal.Windows.cs | 170 +++++------------- .../UnitTests/Fakes/FakeNameResolutionPal.cs | 2 +- 5 files changed, 51 insertions(+), 132 deletions(-) diff --git a/src/Common/src/Interop/Windows/kernel32/Interop.LoadLibraryEx.cs b/src/Common/src/Interop/Windows/kernel32/Interop.LoadLibraryEx.cs index 4ba2fd65a453..d24d4d3c6a88 100644 --- a/src/Common/src/Interop/Windows/kernel32/Interop.LoadLibraryEx.cs +++ b/src/Common/src/Interop/Windows/kernel32/Interop.LoadLibraryEx.cs @@ -12,6 +12,7 @@ internal partial class Interop internal partial class Kernel32 { public const int LOAD_LIBRARY_AS_DATAFILE = 0x00000002; + public const int LOAD_LIBRARY_SEARCH_SYSTEM32 = 0x00000800; [DllImport(Libraries.Kernel32, ExactSpelling = true, CharSet = CharSet.Unicode, SetLastError = true)] public static extern SafeLibraryHandle LoadLibraryExW([In] string lpwLibFileName, [In] IntPtr hFile, [In] uint dwFlags); diff --git a/src/System.Net.NameResolution/src/System/Net/DNS.cs b/src/System.Net.NameResolution/src/System/Net/DNS.cs index d84edd635a59..f15f811f6143 100644 --- a/src/System.Net.NameResolution/src/System/Net/DNS.cs +++ b/src/System.Net.NameResolution/src/System/Net/DNS.cs @@ -325,16 +325,12 @@ private static IAsyncResult HostResolutionBeginHelper(string hostName, bool just // Set up the context, possibly flow. asyncResult.StartPostingAsyncOp(false); - // GetHostByName is used instead of GetAddrInfo if ipv6 is not supported. - // See the comment in InternalGetHostByName for further explanation. - bool useGetHostByName = !includeIPv6 && !SocketProtocolSupportPal.OSSupportsIPv6; - // If the OS supports it and 'hostName' is not an IP Address, resolve the name asynchronously // instead of calling the synchronous version in the ThreadPool. - if (NameResolutionPal.SupportsGetAddrInfoAsync && ipAddress == null && !useGetHostByName) + if (NameResolutionPal.SupportsGetAddrInfoAsync && ipAddress == null) { ValidateHostName(hostName); - NameResolutionPal.GetAddrInfoAsync(hostName, asyncResult); + NameResolutionPal.GetAddrInfoAsync(asyncResult); } else { diff --git a/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Unix.cs b/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Unix.cs index 46f70805f072..e823898e4d39 100644 --- a/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Unix.cs +++ b/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Unix.cs @@ -196,7 +196,7 @@ public static unsafe SocketError TryGetAddrInfo(string name, out IPHostEntry hos return SocketError.Success; } - internal static void GetAddrInfoAsync(string name, DnsResolveAsyncResult asyncResult) + internal static void GetAddrInfoAsync(DnsResolveAsyncResult asyncResult) { throw new NotSupportedException(); } diff --git a/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Windows.cs b/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Windows.cs index 96ab2cbc232a..af93851a06bb 100644 --- a/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Windows.cs +++ b/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Windows.cs @@ -397,7 +397,7 @@ public static void EnsureSocketsAreInitialized() private static bool GetAddrInfoExSupportsOverlapped() { - using (SafeLibraryHandle libHandle = Interop.Kernel32.LoadLibraryExW(Interop.Libraries.Ws2_32, IntPtr.Zero, 0)) + using (SafeLibraryHandle libHandle = Interop.Kernel32.LoadLibraryExW(Interop.Libraries.Ws2_32, IntPtr.Zero, Interop.Kernel32.LOAD_LIBRARY_SEARCH_SYSTEM32)) { if (libHandle.IsInvalid) return false; @@ -408,16 +408,27 @@ private static bool GetAddrInfoExSupportsOverlapped() } } - public static unsafe void GetAddrInfoAsync(string name, DnsResolveAsyncResult asyncResult) + public static unsafe void GetAddrInfoAsync(DnsResolveAsyncResult asyncResult) { - GetAddrInfoExContext* context = GetAddrInfoExContext.AllocateContext(name, asyncResult); + GetAddrInfoExContext* context = GetAddrInfoExContext.AllocateContext(); + + try + { + var state = new GetAddrInfoExState(asyncResult); + context->QueryStateHandle = state.CreateHandle(); + } + catch + { + GetAddrInfoExContext.FreeContext(context); + throw; + } AddressInfoEx hints = new AddressInfoEx(); hints.ai_flags = AddressInfoHints.AI_CANONNAME; hints.ai_family = AddressFamily.Unspecified; // Gets all address families SocketError errorCode = - (SocketError)Interop.Winsock.GetAddrInfoExW(name, null, 0 /* NS_ALL*/, IntPtr.Zero, ref hints, out context->Result, IntPtr.Zero, ref context->Overlapped, s_getAddrInfoExCallback, out context->CancelHandle); + (SocketError)Interop.Winsock.GetAddrInfoExW(asyncResult.HostName, null, 0 /* NS_ALL*/, IntPtr.Zero, ref hints, out context->Result, IntPtr.Zero, ref context->Overlapped, s_getAddrInfoExCallback, out context->CancelHandle); if (errorCode != SocketError.IOPending) ProcessResult(errorCode, context); @@ -433,17 +444,10 @@ private static unsafe void GetAddressInfoExCallback([In] int error, [In] int byt private static unsafe void ProcessResult(SocketError errorCode, GetAddrInfoExContext* context) { - GetAddrInfoExState state = context->GetQueryState(); - - // 'state' may be null or 'SetCallbackStartedOrCanceled' may return false if - // the AppDomain started unloading before the callback was raised. - // In this case we don't need to free the context, it will be done in - // GetAddrInfoExState's finalizer. - if (state == null || !state.SetCallbackStartedOrCanceled()) - return; - try { + GetAddrInfoExState state = GetAddrInfoExState.FromHandle(context->QueryStateHandle); + if (errorCode != SocketError.Success) { state.CompleteAsyncResult(new SocketException((int)errorCode)); @@ -477,7 +481,7 @@ private static unsafe void ProcessResult(SocketError errorCode, GetAddrInfoExCon } if (canonicalName == null) - canonicalName = state.HostAddress; + canonicalName = state.HostName; state.CompleteAsyncResult(new IPHostEntry { @@ -488,7 +492,7 @@ private static unsafe void ProcessResult(SocketError errorCode, GetAddrInfoExCon } finally { - state.Dispose(); + GetAddrInfoExContext.FreeContext(context); } } @@ -509,157 +513,75 @@ private static unsafe IPAddress CreateIPv6Address(ReadOnlySpan socketAddre #region GetAddrInfoAsync Helper Classes - private sealed unsafe class GetAddrInfoExState : IDisposable + // + // Warning: If this ever ported to NETFX, AppDomain unloads needs to be handled + // to protect against AppDomainUnloadException if there are pending operations. + // + + private sealed class GetAddrInfoExState { - private IntPtr _context; - private int _callbackStartedOrCanceled; private DnsResolveAsyncResult _asyncResult; + private object _result; - public string HostAddress { get; } + public string HostName => _asyncResult.HostName; - public GetAddrInfoExState(string hostAddress, DnsResolveAsyncResult asyncResult) + public GetAddrInfoExState(DnsResolveAsyncResult asyncResult) { - HostAddress = hostAddress; _asyncResult = asyncResult; } - public void SetQueryContext(IntPtr context) - { - _context = context; - } - - public bool SetCallbackStartedOrCanceled() - { - return Interlocked.Exchange(ref _callbackStartedOrCanceled, 1) == 0; - } - public void CompleteAsyncResult(object o) { // We don't want to expose the GetAddrInfoEx callback thread to user code. + // The callback occurs in a native windows thread pool. - var state = Tuple.Create(_asyncResult, o); + _result = o; Task.Factory.StartNew(s => { - var t = (Tuple)s; - t.Item1.InvokeCallback(t.Item2); - }, state, CancellationToken.None, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default); + var self = (GetAddrInfoExState)s; + self._asyncResult.InvokeCallback(self._result); + }, this, CancellationToken.None, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default); } - ~GetAddrInfoExState() + public IntPtr CreateHandle() { - // The finalizer should only be called when the AppDomain is unloading while there was a pending operation. - // Calling GetAddrInfoExCancel will invoke the callback inline with WSA_E_CANCELLED error code. - - // If the callback is already invoked, let the traditional Dispose clear the resources. - // This should not be possible, because the finalizer would not have been called if the object was still alive. - // This is an extra protection in case the object was somehow resurrected. - if (!SetCallbackStartedOrCanceled()) - return; - - ReleaseResources(cancelQuery: true); + GCHandle handle = GCHandle.Alloc(this, GCHandleType.Normal); + return GCHandle.ToIntPtr(handle); } - public void Dispose() + public static GetAddrInfoExState FromHandle(IntPtr handle) { - GC.SuppressFinalize(this); - ReleaseResources(cancelQuery: false); - } + GCHandle gcHandle = GCHandle.FromIntPtr(handle); + var state = (GetAddrInfoExState)gcHandle.Target; + gcHandle.Free(); - private void ReleaseResources(bool cancelQuery) - { - IntPtr ptr = Interlocked.Exchange(ref _context, IntPtr.Zero); - - if (ptr == IntPtr.Zero) - return; - - var context = (GetAddrInfoExContext*)ptr; - - if (cancelQuery) - context->CancelQuery(); - - GetAddrInfoExContext.FreeContext(context); + return state; } } [StructLayout(LayoutKind.Sequential)] private unsafe struct GetAddrInfoExContext { - private static readonly int Size = Marshal.SizeOf(); + private static readonly int Size = sizeof(GetAddrInfoExContext); public NativeOverlapped Overlapped; public AddressInfoEx* Result; - public IntPtr QueryStateHandle; public IntPtr CancelHandle; + public IntPtr QueryStateHandle; - public GetAddrInfoExContext(GetAddrInfoExState state) - { - Overlapped = new NativeOverlapped(); - Result = null; - - GCHandle handle = GCHandle.Alloc(state, GCHandleType.Normal); - QueryStateHandle = GCHandle.ToIntPtr(handle); - - CancelHandle = IntPtr.Zero; - } - - public GetAddrInfoExState GetQueryState() - { - IntPtr stateHandle = Interlocked.Exchange(ref QueryStateHandle, IntPtr.Zero); - - if (stateHandle == IntPtr.Zero) - return null; - - GCHandle handle = GCHandle.FromIntPtr(stateHandle); - - if (!handle.IsAllocated) - return null; - - var state = (GetAddrInfoExState)handle.Target; - handle.Free(); - - return state; - } - - public void CancelQuery() - { - if (CancelHandle != IntPtr.Zero) - { - Interop.Winsock.GetAddrInfoExCancel(ref CancelHandle); - CancelHandle = IntPtr.Zero; - } - } - - public static GetAddrInfoExContext* AllocateContext(string hostAddress, DnsResolveAsyncResult asyncResult) + public static GetAddrInfoExContext* AllocateContext() { - GetAddrInfoExContext* context = (GetAddrInfoExContext*)Marshal.AllocHGlobal(Size); + var context = (GetAddrInfoExContext*)Marshal.AllocHGlobal(Size); + *context = new GetAddrInfoExContext(); - try - { - GetAddrInfoExState state = new GetAddrInfoExState(hostAddress, asyncResult); - - *context = new GetAddrInfoExContext(state); - state.SetQueryContext((IntPtr)context); - } - catch (OutOfMemoryException) - { - Marshal.FreeHGlobal((IntPtr)context); - throw; - } - return context; } public static void FreeContext(GetAddrInfoExContext* context) { if (context->Result != null) - { Interop.Winsock.FreeAddrInfoEx(context->Result); - context->Result = null; - } - - // There is no need to free the GCHandle held in 'QueryStateHandle' - // The handle is freed by 'GetQueryState' unless the AppDomain is unloaded. Marshal.FreeHGlobal((IntPtr)context); } diff --git a/src/System.Net.NameResolution/tests/UnitTests/Fakes/FakeNameResolutionPal.cs b/src/System.Net.NameResolution/tests/UnitTests/Fakes/FakeNameResolutionPal.cs index a3b44a097889..69d1bc992961 100644 --- a/src/System.Net.NameResolution/tests/UnitTests/Fakes/FakeNameResolutionPal.cs +++ b/src/System.Net.NameResolution/tests/UnitTests/Fakes/FakeNameResolutionPal.cs @@ -51,7 +51,7 @@ internal static string TryGetNameInfo(IPAddress address, out SocketError errorCo throw new NotImplementedException(); } - internal static void GetAddrInfoAsync(string name, DnsResolveAsyncResult asyncResult) + internal static void GetAddrInfoAsync(DnsResolveAsyncResult asyncResult) { throw new NotImplementedException(); } From 524b5fa61bad731531d78824db3aeaeb9dd735e8 Mon Sep 17 00:00:00 2001 From: JeffCyr Date: Sun, 11 Feb 2018 17:59:01 -0500 Subject: [PATCH 10/10] cleanup --- .../Interop/Windows/Winsock/Interop.GetAddrInfoExW.cs | 5 ++--- src/Common/src/System/Net/SocketAddressPal.Unix.cs | 10 ---------- src/Common/src/System/Net/SocketAddressPal.Windows.cs | 10 ---------- .../src/System/Net/NameResolutionPal.Windows.cs | 8 ++++---- 4 files changed, 6 insertions(+), 27 deletions(-) diff --git a/src/Common/src/Interop/Windows/Winsock/Interop.GetAddrInfoExW.cs b/src/Common/src/Interop/Windows/Winsock/Interop.GetAddrInfoExW.cs index 902799d0d2ff..cb2070de81e5 100644 --- a/src/Common/src/Interop/Windows/Winsock/Interop.GetAddrInfoExW.cs +++ b/src/Common/src/Interop/Windows/Winsock/Interop.GetAddrInfoExW.cs @@ -11,6 +11,8 @@ internal static partial class Interop { internal static partial class Winsock { + internal const string GetAddrInfoExCancelFunctionName = "GetAddrInfoExCancel"; + internal unsafe delegate void LPLOOKUPSERVICE_COMPLETION_ROUTINE([In] int dwError, [In] int dwBytes, [In] NativeOverlapped* lpOverlapped); [DllImport(Interop.Libraries.Ws2_32, ExactSpelling = true, CharSet = CharSet.Unicode, SetLastError = true)] @@ -29,9 +31,6 @@ [Out] out IntPtr lpNameHandle [DllImport("ws2_32.dll", ExactSpelling = true, SetLastError = true)] internal static extern unsafe void FreeAddrInfoEx([In] AddressInfoEx* pAddrInfo); - - [DllImport("ws2_32.dll", ExactSpelling = true, SetLastError = true)] - internal static extern int GetAddrInfoExCancel([In] ref IntPtr lpHandle); } } diff --git a/src/Common/src/System/Net/SocketAddressPal.Unix.cs b/src/Common/src/System/Net/SocketAddressPal.Unix.cs index 082a8babbc3a..94360635e210 100644 --- a/src/Common/src/System/Net/SocketAddressPal.Unix.cs +++ b/src/Common/src/System/Net/SocketAddressPal.Unix.cs @@ -102,11 +102,6 @@ public static unsafe void SetPort(byte[] buffer, ushort port) ThrowOnFailure(err); } - public static unsafe uint GetIPv4Address(byte[] buffer) - { - return GetIPv4Address(new ReadOnlySpan(buffer)); - } - public static unsafe uint GetIPv4Address(ReadOnlySpan buffer) { uint ipAddress; @@ -120,11 +115,6 @@ public static unsafe uint GetIPv4Address(ReadOnlySpan buffer) return ipAddress; } - public static void GetIPv6Address(byte[] buffer, Span address, out uint scope) - { - GetIPv6Address(new ReadOnlySpan(buffer), address, out scope); - } - public static unsafe void GetIPv6Address(ReadOnlySpan buffer, Span address, out uint scope) { uint localScope; diff --git a/src/Common/src/System/Net/SocketAddressPal.Windows.cs b/src/Common/src/System/Net/SocketAddressPal.Windows.cs index 1b66d505eed5..404a381ae8d9 100644 --- a/src/Common/src/System/Net/SocketAddressPal.Windows.cs +++ b/src/Common/src/System/Net/SocketAddressPal.Windows.cs @@ -38,11 +38,6 @@ public static unsafe void SetPort(byte[] buffer, ushort port) port.HostToNetworkBytes(buffer, 2); } - public static unsafe uint GetIPv4Address(byte[] buffer) - { - return GetIPv4Address(new ReadOnlySpan(buffer)); - } - public static unsafe uint GetIPv4Address(ReadOnlySpan buffer) { unchecked @@ -54,11 +49,6 @@ public static unsafe uint GetIPv4Address(ReadOnlySpan buffer) } } - public static void GetIPv6Address(byte[] buffer, Span address, out uint scope) - { - GetIPv6Address(new ReadOnlySpan(buffer), address, out scope); - } - public static unsafe void GetIPv6Address(ReadOnlySpan buffer, Span address, out uint scope) { for (int i = 0; i < address.Length; i++) diff --git a/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Windows.cs b/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Windows.cs index af93851a06bb..1b01aa44ecca 100644 --- a/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Windows.cs +++ b/src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Windows.cs @@ -404,7 +404,7 @@ private static bool GetAddrInfoExSupportsOverlapped() // We can't just check that 'GetAddrInfoEx' exists, because it existed before supporting overlapped. // The existance of 'GetAddrInfoExCancel' indicates that overlapped is supported. - return Interop.Kernel32.GetProcAddress(libHandle, nameof(Interop.Winsock.GetAddrInfoExCancel)) != IntPtr.Zero; + return Interop.Kernel32.GetProcAddress(libHandle, Interop.Winsock.GetAddrInfoExCancelFunctionName) != IntPtr.Zero; } } @@ -446,7 +446,7 @@ private static unsafe void ProcessResult(SocketError errorCode, GetAddrInfoExCon { try { - GetAddrInfoExState state = GetAddrInfoExState.FromHandle(context->QueryStateHandle); + GetAddrInfoExState state = GetAddrInfoExState.FromHandleAndFree(context->QueryStateHandle); if (errorCode != SocketError.Success) { @@ -550,7 +550,7 @@ public IntPtr CreateHandle() return GCHandle.ToIntPtr(handle); } - public static GetAddrInfoExState FromHandle(IntPtr handle) + public static GetAddrInfoExState FromHandleAndFree(IntPtr handle) { GCHandle gcHandle = GCHandle.FromIntPtr(handle); var state = (GetAddrInfoExState)gcHandle.Target; @@ -573,7 +573,7 @@ private unsafe struct GetAddrInfoExContext public static GetAddrInfoExContext* AllocateContext() { var context = (GetAddrInfoExContext*)Marshal.AllocHGlobal(Size); - *context = new GetAddrInfoExContext(); + *context = default; return context; }