From c112de11a2c4c6d7a5e228cf2bce607184a1a55a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ahmet=20=C4=B0brahim=20AKSOY?= Date: Fri, 26 Aug 2022 06:34:23 +0300 Subject: [PATCH] [Unix]: AcceptAsync with existing AcceptSocket support on Unix (#73499) * Fix: Initial attempt to fix - ut passed * Update: Forgotten save * Update: Clear and Dispose ops on handles * Update: Activate tests for Unix and correct no reuse behavior * Update: Review Changes * Update: Deleted forgotten comment * Update: Review change --- .../System/Net/Sockets/SafeSocketHandle.cs | 2 + .../src/System/Net/Sockets/Socket.Unix.cs | 92 ++++++++++++++++++- .../Net/Sockets/SocketAsyncEventArgs.Unix.cs | 12 +++ .../tests/FunctionalTests/Accept.cs | 4 - 4 files changed, 101 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SafeSocketHandle.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SafeSocketHandle.cs index 73f8a2c6e9c..c8b2cfdccb6 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SafeSocketHandle.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SafeSocketHandle.cs @@ -53,6 +53,8 @@ public SafeSocketHandle(IntPtr preexistingHandle, bool ownsHandle) internal bool OwnsHandle { get; } + internal bool HasShutdownSend => _hasShutdownSend; + private bool TryOwnClose() => Interlocked.CompareExchange(ref _ownClose, 1, 0) == 0; diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Unix.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Unix.cs index e8ecb6e499b..000e4f9aee6 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Unix.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Unix.cs @@ -6,6 +6,8 @@ using System.Threading.Tasks; using System.Runtime.Versioning; using Microsoft.Win32.SafeHandles; +using System.Reflection; +using System.Collections; namespace System.Net.Sockets { @@ -166,16 +168,20 @@ private static void ThrowMultiConnectNotSupported() } #pragma warning disable CA1822 - private Socket? GetOrCreateAcceptSocket(Socket? acceptSocket, bool unused, string propertyName, out SafeSocketHandle? handle) + private Socket? GetOrCreateAcceptSocket(Socket? acceptSocket, bool checkDisconnected, string propertyName, out SafeSocketHandle? handle) { - // AcceptSocket is not supported on Unix. - if (acceptSocket != null) + if (acceptSocket != null && acceptSocket._handle.HasShutdownSend) { - throw new PlatformNotSupportedException(SR.PlatformNotSupported_AcceptSocket); + throw new SocketException((int)SocketError.InvalidArgument); + } + + if (acceptSocket != null && acceptSocket._rightEndPoint != null && (!checkDisconnected || !acceptSocket._isDisconnected)) + { + throw new InvalidOperationException(SR.Format(SR.net_sockets_namedmustnotbebound, propertyName)); } handle = null; - return null; + return acceptSocket; } #pragma warning restore CA1822 @@ -228,5 +234,81 @@ private void SendFileInternal(string? fileName, ReadOnlySpan preBuffer, Re Send(postBuffer); } } + + internal void DisposeHandle() + { + _handle.Dispose(); + } + + internal void ClearHandle() + { + _handle = null!; + } + + internal Socket CopyStateFromSource(Socket source) + { + _addressFamily = source._addressFamily; + _closeTimeout = source._closeTimeout; + _disposed = source._disposed; + _handle = source._handle; + _isConnected = source._isConnected; + _isDisconnected = source._isDisconnected; + _isListening = source._isListening; + _nonBlockingConnectInProgress = source._nonBlockingConnectInProgress; + _protocolType = source._protocolType; + _receivingPacketInformation = source._receivingPacketInformation; + _remoteEndPoint = source._remoteEndPoint; + _rightEndPoint = source._rightEndPoint; + _socketType = source._socketType; + _willBlock = source._willBlock; + _willBlockInternal = source._willBlockInternal; + _localEndPoint = source._localEndPoint; + _multiBufferReceiveEventArgs = source._multiBufferReceiveEventArgs; + _multiBufferSendEventArgs = source._multiBufferSendEventArgs; + _pendingConnectRightEndPoint = source._pendingConnectRightEndPoint; + _singleBufferReceiveEventArgs = source._singleBufferReceiveEventArgs; +#if DEBUG + // Try to detect if a property gets added that we're not copying correctly. + foreach (PropertyInfo pi in typeof(Socket).GetProperties(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.DeclaredOnly)) + { + object? origValue = pi.GetValue(source); + object? cloneValue = pi.GetValue(this); + + if (origValue is IEnumerable origEnumerable) + { + IEnumerable? cloneEnumerable = cloneValue as IEnumerable; + Debug.Assert(cloneEnumerable != null, $"{pi.Name}. Expected enumerable cloned value."); + + IEnumerator e1 = origEnumerable.GetEnumerator(); + try + { + IEnumerator e2 = cloneEnumerable.GetEnumerator(); + try + { + while (e1.MoveNext()) + { + Debug.Assert(e2.MoveNext(), $"{pi.Name}. Cloned enumerator too short."); + Debug.Assert(Equals(e1.Current, e2.Current), $"{pi.Name}. Cloned enumerator's values don't match."); + } + Debug.Assert(!e2.MoveNext(), $"{pi.Name}. Cloned enumerator too long."); + } + finally + { + (e2 as IDisposable)?.Dispose(); + } + } + finally + { + (e1 as IDisposable)?.Dispose(); + } + } + else + { + Debug.Assert(Equals(origValue, cloneValue), $"{pi.Name}. Expected: {origValue}, Actual: {cloneValue}"); + } + } +#endif + return this; + } } } diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.Unix.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.Unix.cs index df92fa76e22..7f946d57d12 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.Unix.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.Unix.cs @@ -335,9 +335,21 @@ internal void LogBuffer(int size) private SocketError FinishOperationAccept(Internals.SocketAddress remoteSocketAddress) { System.Buffer.BlockCopy(_acceptBuffer!, 0, remoteSocketAddress.Buffer, 0, _acceptAddressBufferCount); + Socket? sukru = _acceptSocket; _acceptSocket = _currentSocket!.CreateAcceptSocket( SocketPal.CreateSocket(_acceptedFileDescriptor), _currentSocket._rightEndPoint!.Create(remoteSocketAddress)); + if (sukru != null) + { + sukru.DisposeHandle(); + sukru.CopyStateFromSource(_acceptSocket); + // We keep this socket to make clean-up. + Socket temp = _acceptSocket; + _acceptSocket = sukru; + temp.ClearHandle(); + temp.Dispose(); + GC.SuppressFinalize(temp); + } return SocketError.Success; } diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs index f0fbcde1e82..57c4d32a973 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/Accept.cs @@ -145,7 +145,6 @@ public async Task Accept_ConcurrentAcceptsAfterConnects_Success(int numberAccept [OuterLoop] [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/1483", TestPlatforms.AnyUnix)] public async Task Accept_WithTargetSocket_Success() { if (!SupportsAcceptIntoExistingSocket) @@ -167,7 +166,6 @@ public async Task Accept_WithTargetSocket_Success() } } - [ActiveIssue("https://github.com/dotnet/runtime/issues/1483", TestPlatforms.AnyUnix)] [OuterLoop] [Theory] [InlineData(false)] @@ -222,7 +220,6 @@ public async Task Accept_WithTargetSocket_ReuseAfterDisconnect_Success(bool reus [OuterLoop] [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/1483", TestPlatforms.AnyUnix)] public async Task Accept_WithAlreadyBoundTargetSocket_Fails() { if (!SupportsAcceptIntoExistingSocket) @@ -243,7 +240,6 @@ public async Task Accept_WithAlreadyBoundTargetSocket_Fails() [OuterLoop] [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/1483", TestPlatforms.AnyUnix)] public async Task Accept_WithInUseTargetSocket_Fails() { if (!SupportsAcceptIntoExistingSocket) -- GitLab