Socket: don't perform RST close on Dispose when user called Shutdown (#…

…41250)

* Socket: don't perform RST close on Dispose when user called Shutdown

* Add test

* PR feedback

* Remove confusing 'canAbort' local

* Improve comments

src/System.Net.Sockets/src/System/Net/Sockets/SafeSocketHandle.Unix.cs

@@ -366,7 +366,8 @@ internal static unsafe InnerSafeCloseSocket Accept(SafeSocketHandle socketHandle
                 return res;
             }
 
-            internal unsafe bool TryUnblockSocket(bool abortive)
+            /// <returns>Returns whether operations were canceled.</returns>
+            internal unsafe bool TryUnblockSocket(bool abortive, bool hasShutdownSend)
             {
                 // Calling 'close' on a socket that has pending blocking calls (e.g. recv, send, accept, ...)
                 // may block indefinitely. This is a best-effort attempt to not get blocked and make those operations return.
@@ -392,7 +393,9 @@ internal unsafe bool TryUnblockSocket(bool abortive)
                 Interop.Error err = Interop.Sys.GetSockOpt(this, SocketOptionLevel.Socket, SocketOptionName.Type, (byte*)&type, &optLen);
                 if (err == Interop.Error.SUCCESS)
                 {
-                    if (type == (int)SocketType.Stream)
+                    // For TCP (SocketType.Stream), perform an abortive close.
+                    // Unless the user requested a normal close using Socket.Shutdown.
+                    if (type == (int)SocketType.Stream && !hasShutdownSend)
                     {
                         Interop.Sys.Disconnect(this);
                     }
@@ -402,7 +405,6 @@ internal unsafe bool TryUnblockSocket(bool abortive)
                     }
                 }
 
-                // We've cancelled on-going operations, return true to cause an abortive close.
                 return true;
             }
         }

src/System.Net.Sockets/src/System/Net/Sockets/SafeSocketHandle.Windows.cs

@@ -236,10 +236,10 @@ internal static InnerSafeCloseSocket Accept(SafeSocketHandle socketHandle, byte[
                 return result;
             }
 
-            internal unsafe bool TryUnblockSocket(bool abortive)
+            /// <returns>Returns whether operations were canceled.</returns>
+            internal unsafe bool TryUnblockSocket(bool abortive, bool hasShutdownSend)
             {
                 // Try to cancel all pending IO.
-                // If we've canceled operations, we return true to cause an abortive close.
                 return Interop.Kernel32.CancelIoEx(this, null);
             }
         }

src/System.Net.Sockets/src/System/Net/Sockets/SafeSocketHandle.cs

@@ -37,6 +37,16 @@ private SafeSocketHandle() : base(true) { }
 #if DEBUG
         private InnerSafeCloseSocket _innerSocketCopy;
 #endif
+        private bool _hasShutdownSend;
+
+        internal void TrackShutdown(SocketShutdown how)
+        {
+            if (how == SocketShutdown.Send ||
+                how == SocketShutdown.Both)
+            {
+                _hasShutdownSend = true;
+            }
+        }
 
         public override bool IsInvalid
         {
@@ -171,6 +181,8 @@ internal void CloseAsIs(bool abortive)
                 Dispose();
                 if (innerSocket != null)
                 {
+                    bool canceledOperations = false;
+
                     // Wait until it's safe.
                     SpinWait sw = new SpinWait();
                     while (!_released)
@@ -179,11 +191,18 @@ internal void CloseAsIs(bool abortive)
                         // Try to make those on-going calls return.
                         // On Linux, TryUnblockSocket will unblock current operations but it doesn't prevent
                         // a new one from starting. So we must call TryUnblockSocket multiple times.
-                        abortive |= innerSocket.TryUnblockSocket(abortive);
+                        canceledOperations |= innerSocket.TryUnblockSocket(abortive, _hasShutdownSend);
                         sw.SpinOnce();
                     }
 
-                    abortive |= DoReleaseHandle();
+                    canceledOperations |= DoReleaseHandle();
+
+                    // In case we cancel operations, switch to an abortive close.
+                    // Unless the user requested a normal close using Socket.Shutdown.
+                    if (canceledOperations && !_hasShutdownSend)
+                    {
+                        abortive = true;
+                    }
 
                     innerSocket.Close(abortive);
                 }

src/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Unix.cs

@@ -1535,6 +1535,7 @@ public static SocketError Shutdown(SafeSocketHandle handle, bool isConnected, bo
             Interop.Error err = Interop.Sys.Shutdown(handle, how);
             if (err == Interop.Error.SUCCESS)
             {
+                handle.TrackShutdown(how);
                 return SocketError.Success;
             }
 
@@ -1543,6 +1544,7 @@ public static SocketError Shutdown(SafeSocketHandle handle, bool isConnected, bo
             // has reached the CLOSE state. Ignoring the error matches Winsock behavior.
             if (err == Interop.Error.ENOTCONN && (isConnected || isDisconnected))
             {
+                handle.TrackShutdown(how);
                 return SocketError.Success;
             }
 

src/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Windows.cs

@@ -891,6 +891,7 @@ public static SocketError Shutdown(SafeSocketHandle handle, bool isConnected, bo
             SocketError err = Interop.Winsock.shutdown(handle, (int)how);
             if (err != SocketError.SocketError)
             {
+                handle.TrackShutdown(how);
                 return SocketError.Success;
             }
 

src/System.Net.Sockets/tests/FunctionalTests/SendReceive.cs

@@ -1106,6 +1106,42 @@ await RetryHelper.ExecuteAsync(async () =>
                 }
             }, maxAttempts: 10);
         }
+
+        [Fact]
+        public async Task TcpPeerReceivesFinOnShutdownWithPendingData()
+        {
+            // We try this a couple of times to deal with a timing race: if the Dispose happens
+            // before the send is started, no data is sent.
+            int msDelay = 100;
+            byte[] hugeBuffer = new byte[100_000_000];
+            byte[] receiveBuffer = new byte[1024];
+            await RetryHelper.ExecuteAsync(async () =>
+            {
+                (Socket socket1, Socket socket2) = CreateConnectedSocketPair();
+                using (socket1)
+                using (socket2)
+                {
+                    // socket1: send a huge amount of data, then Shutdown and Dispose before the peer starts reading.
+                    Task sendTask = SendAsync(socket1, hugeBuffer);
+                    // Wait a little so the operation is started.
+                    await Task.Delay(msDelay);
+                    msDelay *= 2;
+                    socket1.Shutdown(SocketShutdown.Both);
+                    socket1.Dispose();
+
+                    // socket2: read until FIN.
+                    int receivedTotal = 0;
+                    int received;
+                    do
+                    {
+                        received = await ReceiveAsync(socket2, receiveBuffer);
+                        receivedTotal += received;
+                    } while (received != 0);
+
+                    Assert.NotEqual(0, receivedTotal);
+                }
+            }, maxAttempts: 10);
+        }
     }
 
     public class SendReceive