Socket: improve cross-platform behavior on Dispose (#38804)

* Add tests * Don't skip SyncSendFileGetsCanceledByDispose on .NET Framework * Implement using code from previous PR * Update SocketErrors when CleanedUp for APM APIs * Cleanup SO_TYPE socket length handling (Fixes https://github.com/dotnet/corefx/issues/38750) * SyncSendFileGetsCanceledByDispose: remove SkipTargetFramework NetFramework * PR feedback * Fix comment typo * Also wait for async operations, and wait longer on each retry * clear sockaddr using memset * Fix build * Only perform CLOEXEC check for non-abortive close
dotnet · Jul 17, 2019 · 9b2f7a8 · 9b2f7a8
1 parent b5995e6
commit 9b2f7a8
Show file tree

Hide file tree

Showing 38 changed files with 788 additions and 186 deletions.
diff --git a/...Unix/System.Native/Interop.Fcntl.SetFD.cs → .../Unix/System.Native/Interop.Disconnect.cs b/...Unix/System.Native/Interop.Fcntl.SetFD.cs → .../Unix/System.Native/Interop.Disconnect.cs
@@ -4,16 +4,12 @@
 
 using System;
 using System.Runtime.InteropServices;
-using Microsoft.Win32.SafeHandles;
 
 internal static partial class Interop
 {
     internal static partial class Sys
     {
-        internal static partial class Fcntl
-        {
-            [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_FcntlSetFD", SetLastError=true)]
-            internal static extern int SetFD(SafeHandle fd, int flags);
-        }
+        [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_Disconnect")]
+        internal static extern unsafe Error Disconnect(SafeHandle socket);
     }
 }
diff --git a/src/Common/src/Interop/Unix/System.Native/Interop.Fcntl.cs b/src/Common/src/Interop/Unix/System.Native/Interop.Fcntl.cs
@@ -9,13 +9,19 @@ internal static partial class Interop
 {
     internal static partial class Sys
     {
-        internal static class Fcntl
+        internal static partial class Fcntl
         {
             [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_FcntlSetIsNonBlocking", SetLastError = true)]
             internal static extern int DangerousSetIsNonBlocking(IntPtr fd, int isNonBlocking);
 
             [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_FcntlSetIsNonBlocking", SetLastError=true)]
             internal static extern int SetIsNonBlocking(SafeHandle fd, int isNonBlocking);
+
+            [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_FcntlSetFD", SetLastError=true)]
+            internal static extern int SetFD(SafeHandle fd, int flags);
+
+            [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_FcntlGetFD", SetLastError=true)]
+            internal static extern int GetFD(SafeHandle fd);
         }
     }
 }
diff --git a/src/Common/src/Interop/Windows/WinSock/Interop.WSAConnect.cs b/src/Common/src/Interop/Windows/WinSock/Interop.WSAConnect.cs
@@ -10,10 +10,9 @@ internal static partial class Interop
 {
     internal static partial class Winsock
     {
-        // This function is always potentially blocking so it uses an IntPtr.
         [DllImport(Interop.Libraries.Ws2_32, SetLastError = true)]
         internal static extern SocketError WSAConnect(
-            [In] IntPtr socketHandle,
+            SafeSocketHandle socketHandle,
             [In] byte[] socketAddress,
             [In] int socketAddressSize,
             [In] IntPtr inBuffer,

diff --git a/src/Common/src/Interop/Windows/WinSock/Interop.WSAIoctl.cs b/src/Common/src/Interop/Windows/WinSock/Interop.WSAIoctl.cs
@@ -13,7 +13,7 @@ internal static partial class Winsock
         // Used with SIOGETEXTENSIONFUNCTIONPOINTER - we're assuming that will never block.
         [DllImport(Interop.Libraries.Ws2_32, SetLastError = true)]
         internal static extern SocketError WSAIoctl(
-            [In] SafeSocketHandle socketHandle,
+            SafeSocketHandle socketHandle,
             [In] int ioControlCode,
             [In, Out] ref Guid guid,
             [In] int guidSize,
@@ -25,7 +25,7 @@ internal static extern SocketError WSAIoctl(
 
         [DllImport(Interop.Libraries.Ws2_32, SetLastError = true, EntryPoint = "WSAIoctl")]
         internal static extern SocketError WSAIoctl_Blocking(
-            [In] IntPtr socketHandle,
+            SafeSocketHandle socketHandle,
             [In] int ioControlCode,
             [In] byte[] inBuffer,
             [In] int inBufferSize,

diff --git a/src/Common/src/Interop/Windows/WinSock/Interop.WSARecv.cs b/src/Common/src/Interop/Windows/WinSock/Interop.WSARecv.cs
@@ -22,16 +22,6 @@ internal static unsafe extern SocketError WSARecv(
             NativeOverlapped* overlapped,
             IntPtr completionRoutine);
 
-        [DllImport(Interop.Libraries.Ws2_32, SetLastError = true)]
-        internal static unsafe extern SocketError WSARecv(
-            IntPtr socketHandle,
-            WSABuffer* buffer,
-            int bufferCount,
-            out int bytesTransferred,
-            ref SocketFlags socketFlags,
-            NativeOverlapped* overlapped,
-            IntPtr completionRoutine);
-
         internal static unsafe SocketError WSARecv(
             SafeHandle socketHandle,
             ref WSABuffer buffer,
@@ -63,21 +53,5 @@ internal static unsafe SocketError WSARecv(
                 return WSARecv(socketHandle, buffersPtr, bufferCount, out bytesTransferred, ref socketFlags, overlapped, completionRoutine);
             }
         }
-
-        internal static unsafe SocketError WSARecv(
-            IntPtr socketHandle,
-            Span<WSABuffer> buffers,
-            int bufferCount,
-            out int bytesTransferred,
-            ref SocketFlags socketFlags,
-            NativeOverlapped* overlapped,
-            IntPtr completionRoutine)
-        {
-            Debug.Assert(!buffers.IsEmpty);
-            fixed (WSABuffer* buffersPtr = &MemoryMarshal.GetReference(buffers))
-            {
-                return WSARecv(socketHandle, buffersPtr, bufferCount, out bytesTransferred, ref socketFlags, overlapped, completionRoutine);
-            }
-        }
     }
 }
diff --git a/src/Common/src/Interop/Windows/WinSock/Interop.WSASend.cs b/src/Common/src/Interop/Windows/WinSock/Interop.WSASend.cs
@@ -12,16 +12,6 @@ internal static partial class Interop
 {
     internal static partial class Winsock
     {
-        [DllImport(Interop.Libraries.Ws2_32, SetLastError = true)]
-        internal static extern unsafe SocketError WSASend(
-            IntPtr socketHandle,
-            WSABuffer* buffers,
-            int bufferCount,
-            out int bytesTransferred,
-            SocketFlags socketFlags,
-            NativeOverlapped* overlapped,
-            IntPtr completionRoutine);
-
         [DllImport(Interop.Libraries.Ws2_32, SetLastError = true)]
         internal static extern unsafe SocketError WSASend(
             SafeHandle socketHandle,
@@ -63,21 +53,5 @@ internal static unsafe SocketError WSASend(
                 return WSASend(socketHandle, buffersPtr, bufferCount, out bytesTransferred, socketFlags, overlapped, completionRoutine);
             }
         }
-
-        internal static unsafe SocketError WSASend(
-            IntPtr socketHandle,
-            Span<WSABuffer> buffers,
-            int bufferCount,
-            out int bytesTransferred,
-            SocketFlags socketFlags,
-            NativeOverlapped* overlapped,
-            IntPtr completionRoutine)
-        {
-            Debug.Assert(!buffers.IsEmpty);
-            fixed (WSABuffer* buffersPtr = &MemoryMarshal.GetReference(buffers))
-            {
-                return WSASend(socketHandle, buffersPtr, bufferCount, out bytesTransferred, socketFlags, overlapped, completionRoutine);
-            }
-        }
     }
 }
diff --git a/src/Common/src/Interop/Windows/WinSock/Interop.accept.cs b/src/Common/src/Interop/Windows/WinSock/Interop.accept.cs
@@ -10,10 +10,9 @@ internal static partial class Interop
 {
     internal static partial class Winsock
     {
-        // Blocking call - requires IntPtr instead of SafeSocketHandle.
         [DllImport(Interop.Libraries.Ws2_32, ExactSpelling = true, SetLastError = true)]
         internal static extern SafeSocketHandle.InnerSafeCloseSocket accept(
-            [In] IntPtr socketHandle,
+            SafeSocketHandle socketHandle,
             [Out] byte[] socketAddress,
             [In, Out] ref int socketAddressSize);
     }

diff --git a/src/Common/src/Interop/Windows/WinSock/Interop.recv.cs b/src/Common/src/Interop/Windows/WinSock/Interop.recv.cs
@@ -10,10 +10,9 @@ internal static partial class Interop
 {
     internal static partial class Winsock
     {
-        // This method is always blocking, so it uses an IntPtr.
         [DllImport(Interop.Libraries.Ws2_32, SetLastError = true)]
         internal static extern unsafe int recv(
-            [In] IntPtr socketHandle,
+            SafeSocketHandle socketHandle,
             [In] byte* pinnedBuffer,
             [In] int len,
             [In] SocketFlags socketFlags);

diff --git a/src/Common/src/Interop/Windows/WinSock/Interop.recvfrom.cs b/src/Common/src/Interop/Windows/WinSock/Interop.recvfrom.cs
@@ -13,7 +13,7 @@ internal static partial class Winsock
         // This method is always blocking, so it uses an IntPtr.
         [DllImport(Interop.Libraries.Ws2_32, SetLastError = true)]
         internal static extern unsafe int recvfrom(
-            [In] IntPtr socketHandle,
+            SafeSocketHandle socketHandle,
             [In] byte* pinnedBuffer,
             [In] int len,
             [In] SocketFlags socketFlags,

diff --git a/src/Common/src/Interop/Windows/WinSock/Interop.send.cs b/src/Common/src/Interop/Windows/WinSock/Interop.send.cs
@@ -10,10 +10,9 @@ internal static partial class Interop
 {
     internal static partial class Winsock
     {
-        // This method is always blocking, so it uses an IntPtr.
         [DllImport(Interop.Libraries.Ws2_32, SetLastError = true)]
         internal static extern unsafe int send(
-            [In] IntPtr socketHandle,
+            SafeSocketHandle socketHandle,
             [In] byte* pinnedBuffer,
             [In] int len,
             [In] SocketFlags socketFlags);

diff --git a/src/Common/src/Interop/Windows/WinSock/Interop.sendto.cs b/src/Common/src/Interop/Windows/WinSock/Interop.sendto.cs
@@ -10,10 +10,9 @@ internal static partial class Interop
 {
     internal static partial class Winsock
     {
-        // This method is always blocking, so it uses an IntPtr.
         [DllImport(Interop.Libraries.Ws2_32, SetLastError = true)]
         internal static extern unsafe int sendto(
-            [In] IntPtr socketHandle,
+            SafeSocketHandle socketHandle,
             [In] byte* pinnedBuffer,
             [In] int len,
             [In] SocketFlags socketFlags,

diff --git a/src/Native/Unix/Common/pal_config.h.in b/src/Native/Unix/Common/pal_config.h.in
@@ -96,6 +96,7 @@
 #cmakedefine01 HAVE_IN_EXCL_UNLINK
 #cmakedefine01 HAVE_TCP_H_TCP_KEEPALIVE
 #cmakedefine01 HAVE_BUILTIN_MUL_OVERFLOW
+#cmakedefine01 HAVE_DISCONNECTX
 
 // Mac OS X has stat64, but it is deprecated since plain stat now
 // provides the same 64-bit aware struct when targeting OS X > 10.5

diff --git a/src/Native/Unix/System.Native/pal_io.c b/src/Native/Unix/System.Native/pal_io.c
@@ -530,6 +530,11 @@ int32_t SystemNative_FcntlSetFD(intptr_t fd, int32_t flags)
     return result;
 }
 
+int32_t SystemNative_FcntlGetFD(intptr_t fd)
+{
+    return fcntl(ToFileDescriptor(fd), F_GETFD);
+}
+
 int32_t SystemNative_FcntlCanGetSetPipeSz(void)
 {
 #if defined(F_GETPIPE_SZ) && defined(F_SETPIPE_SZ)

diff --git a/src/Native/Unix/System.Native/pal_io.h b/src/Native/Unix/System.Native/pal_io.h
@@ -432,6 +432,13 @@ DLLEXPORT int32_t SystemNative_Pipe(int32_t pipefd[2], // [out] pipefds[0] gets
  */
 DLLEXPORT int32_t SystemNative_FcntlSetFD(intptr_t fd, int32_t flags);
 
+/**
+ * Gets the flags on a file descriptor.
+ *
+ * Returns flags for success; -1 for failure. Sets errno for failure.
+ */
+DLLEXPORT int32_t SystemNative_FcntlGetFD(intptr_t fd);
+
 /**
  * Determines if the current platform supports getting and setting pipe capacity.
  *

diff --git a/src/Native/Unix/System.Native/pal_networking.c b/src/Native/Unix/System.Native/pal_networking.c
@@ -1753,6 +1753,40 @@ static bool TryGetPlatformSocketOption(int32_t socketOptionName, int32_t socketO
     }
 }
 
+static bool TryConvertSocketTypePlatformToPal(int platformSocketType, int32_t* palSocketType)
+{
+    assert(palSocketType != NULL);
+
+    switch (platformSocketType)
+    {
+        case SOCK_STREAM:
+            *palSocketType = SocketType_SOCK_STREAM;
+            return true;
+
+        case SOCK_DGRAM:
+            *palSocketType = SocketType_SOCK_DGRAM;
+            return true;
+
+        case SOCK_RAW:
+            *palSocketType = SocketType_SOCK_RAW;
+            return true;
+
+#ifdef SOCK_RDM
+        case SOCK_RDM:
+            *palSocketType = SocketType_SOCK_RDM;
+            return true;
+#endif
+
+        case SOCK_SEQPACKET:
+            *palSocketType = SocketType_SOCK_SEQPACKET;
+            return true;
+
+        default:
+            *palSocketType = (int32_t)platformSocketType;
+            return false;
+    }
+}
+
 int32_t SystemNative_GetSockOpt(
     intptr_t socket, int32_t socketOptionLevel, int32_t socketOptionName, uint8_t* optionValue, int32_t* optionLen)
 {
@@ -1811,6 +1845,7 @@ int32_t SystemNative_GetSockOpt(
 
     socklen_t optLen = (socklen_t)*optionLen;
     int err = getsockopt(fd, optLevel, optName, optionValue, &optLen);
+
     if (err != 0)
     {
         return SystemNative_ConvertErrorPlatformToPal(errno);
@@ -1827,6 +1862,20 @@ int32_t SystemNative_GetSockOpt(
     }
 #endif
 
+    if (socketOptionLevel == SocketOptionLevel_SOL_SOCKET)
+    {
+        if (socketOptionName == SocketOptionName_SO_TYPE)
+        {
+            if (optLen != sizeof(int) ||             // getsockopt didn't return an int.
+                *optionLen < (int)sizeof(int32_t) || // optionValue can't fit an int32_t.
+                !TryConvertSocketTypePlatformToPal(*(int*)optionValue, (int32_t*)optionValue))
+            {
+                return Error_ENOTSUP;
+            }
+            optLen = sizeof(int32_t);
+        }
+    }
+
     assert(optLen <= (socklen_t)*optionLen);
     *optionLen = (int32_t)optLen;
     return Error_SUCCESS;
@@ -2618,6 +2667,31 @@ void SystemNative_GetDomainSocketSizes(int32_t* pathOffset, int32_t* pathSize, i
     *addressSize = sizeof(domainSocket);
 }
 
+int32_t SystemNative_Disconnect(intptr_t socket)
+{
+    int fd = ToFileDescriptor(socket);
+    int err;
+
+#if defined(__linux__)
+    // On Linux, we can disconnect a socket by connecting to AF_UNSPEC.
+    // For TCP sockets, this causes an abortive close.
+
+    struct sockaddr addr;
+    memset(&addr, 0, sizeof(addr));
+    addr.sa_family = AF_UNSPEC;
+
+    err = connect(fd, &addr, sizeof(addr));
+#elif HAVE_DISCONNECTX
+    // disconnectx causes a FIN close on OSX. It's the best we can do.
+    err = disconnectx(fd, SAE_ASSOCID_ANY, SAE_CONNID_ANY);
+#else
+    // best-effort, this may cause a FIN close.
+    err = shutdown(fd, SHUT_RDWR);
+#endif
+
+    return err == 0 ? Error_SUCCESS : SystemNative_ConvertErrorPlatformToPal(errno);
+}
+
 int32_t SystemNative_SendFile(intptr_t out_fd, intptr_t in_fd, int64_t offset, int64_t count, int64_t* sent)
 {
     assert(sent != NULL);

diff --git a/src/Native/Unix/System.Native/pal_networking.h b/src/Native/Unix/System.Native/pal_networking.h
@@ -420,4 +420,6 @@ DLLEXPORT void SystemNative_GetDomainSocketSizes(int32_t* pathOffset, int32_t* p
 
 DLLEXPORT int32_t SystemNative_SendFile(intptr_t out_fd, intptr_t in_fd, int64_t offset, int64_t count, int64_t* sent);
 
+DLLEXPORT int32_t SystemNative_Disconnect(intptr_t socket);
+
 DLLEXPORT uint32_t SystemNative_InterfaceNameToIndex(char* interfaceName);
diff --git a/src/Native/Unix/configure.cmake b/src/Native/Unix/configure.cmake
@@ -369,6 +369,11 @@ check_symbol_exists(
     "sys/types.h;sys/event.h"
     HAVE_KQUEUE)
 
+check_symbol_exists(
+    disconnectx
+    "sys/socket.h"
+    HAVE_DISCONNECTX)
+
 set(CMAKE_REQUIRED_FLAGS "-Werror -Wsign-conversion")
 check_c_source_compiles(
      "

diff --git a/src/System.Data.OleDb/src/DbBindings.cs → src/System.Data.OleDb/src/DBBindings.cs b/src/System.Data.OleDb/src/DbBindings.cs → src/System.Data.OleDb/src/DBBindings.cs
diff --git a/src/System.IO.MemoryMappedFiles/src/System.IO.MemoryMappedFiles.csproj b/src/System.IO.MemoryMappedFiles/src/System.IO.MemoryMappedFiles.csproj
@@ -122,8 +122,8 @@
     <Compile Include="$(CommonPath)\CoreLib\Interop\Unix\Interop.Errors.cs">
       <Link>Common\CoreLib\Interop\Unix\Interop.Errors.cs</Link>
     </Compile>
-    <Compile Include="$(CommonPath)\Interop\Unix\System.Native\Interop.Fcntl.SetFD.cs">
-      <Link>Common\Interop\Unix\Interop.Fcntl.SetFD.cs</Link>
+    <Compile Include="$(CommonPath)\Interop\Unix\System.Native\Interop.Fcntl.cs">
+      <Link>Common\Interop\Unix\Interop.Fcntl.cs</Link>
     </Compile>
     <Compile Include="$(CommonPath)\CoreLib\Interop\Unix\Interop.IOErrors.cs">
       <Link>Common\CoreLib\Interop\Unix\Interop.IOErrors.cs</Link>

diff --git a/src/System.IO.Pipes/src/System.IO.Pipes.csproj b/src/System.IO.Pipes/src/System.IO.Pipes.csproj
@@ -191,8 +191,8 @@
     <Compile Include="$(CommonPath)\Interop\Unix\System.Native\Interop.Fcntl.Pipe.cs">
       <Link>Common\Interop\Unix\Interop.Fcntl.Pipe.cs</Link>
     </Compile>
-    <Compile Include="$(CommonPath)\Interop\Unix\System.Native\Interop.Fcntl.SetFD.cs">
-      <Link>Common\Interop\Unix\Interop.Fcntl.SetFD.cs</Link>
+    <Compile Include="$(CommonPath)\Interop\Unix\System.Native\Interop.Fcntl.cs">
+      <Link>Common\Interop\Unix\Interop.Fcntl.cs</Link>
     </Compile>
     <Compile Include="$(CommonPath)\CoreLib\Interop\Unix\System.Native\Interop.FLock.cs">
       <Link>Common\Interop\Unix\Interop.FLock.cs</Link>

diff --git a/....Net.NetworkInformation/src/System/Net/NetworkInformation/NetworkAddressChange.Windows.cs b/....Net.NetworkInformation/src/System/Net/NetworkInformation/NetworkAddressChange.Windows.cs
@@ -240,7 +240,7 @@ private static void StartHelper(NetworkAddressChangedEventHandler caller, bool c
                                 true);
 
                             SocketError errorCode = Interop.Winsock.WSAIoctl_Blocking(
-                                s_ipv4Socket.Handle,
+                                s_ipv4Socket.SafeHandle,
                                 (int)IOControlCode.AddressListChange,
                                 null, 0, null, 0,
                                 out int length,
@@ -276,7 +276,7 @@ private static void StartHelper(NetworkAddressChangedEventHandler caller, bool c
                                 true);
 
                             SocketError errorCode = Interop.Winsock.WSAIoctl_Blocking(
-                                s_ipv6Socket.Handle,
+                                s_ipv6Socket.SafeHandle,
                                 (int)IOControlCode.AddressListChange,
                                 null, 0, null, 0,
                                 out int length,