Fix Windows build of ds-ipc-pal-socket.c and make FD_CLOEXEC best effort. (#56141)
authorJohan Lorensson <lateralusx.github@gmail.com>
Mon, 26 Jul 2021 06:35:28 +0000 (08:35 +0200)
committerGitHub <noreply@github.com>
Mon, 26 Jul 2021 06:35:28 +0000 (08:35 +0200)
Up until https://github.com/dotnet/runtime/pull/55850 it was possible
to build using diagnostic server socket PAL on Windows for local testing.
NOTE, this is not a config we build on CI since it is not used by any
products (diagnostic server PAL on Windows uses NamedPipes), but useful
for local testing.

The same PR also included calls to fcntl using FD_CLOEXEC in case platform
doesn't define SOCK_CLOEXEC. Since this is a Linux specific flag, it might not
be defined on several targeted platforms. Failures calling fcntl using
FD_CLOEXEC was also triggering an EP_ASSERT but that is only on debug
builds. Most calls to fcntl using FD_CLOEXEC in runtime PAL layers
(both CoreCLR and MonoVM) doesn't check for errors in this case
and sees the operation as best effort, like done here
https://github.com/dotnet/runtime/blob/b937677e8f8601848d29bc072a93cc0c6e21576d/src/libraries/Native/Unix/System.Native/pal_networking.c#L2499.
We should do the same in ds-ipc-pal-socket.c making sure it won't break
any potential platform not fully supporting it or if we are really concerned
about this error, handle it as real error failing creation of socket, but
that needs to be tested on mobile platforms (not running TCP/IP PAL on CI).

src/native/eventpipe/ds-ipc-pal-socket.c

index 766aa32..2f7cb92 100644 (file)
@@ -323,11 +323,8 @@ ipc_socket_create_uds (DiagnosticsIpc *ipc)
        DS_ENTER_BLOCKING_PAL_SECTION;
        new_socket = socket (ipc->server_address_family, socket_type, 0);
 #ifndef SOCK_CLOEXEC
-       if (new_socket != DS_IPC_INVALID_SOCKET) {
-               if (fcntl (new_socket, F_SETFD, FD_CLOEXEC) == -1) {
-                       EP_ASSERT (!"Failed to set CLOEXEC");
-               }
-       }
+       if (new_socket != DS_IPC_INVALID_SOCKET)
+               fcntl (new_socket, F_SETFD, FD_CLOEXEC); // ignore any failures; this is best effort
 #endif // SOCK_CLOEXEC
        DS_EXIT_BLOCKING_PAL_SECTION;
        return new_socket;
@@ -356,9 +353,9 @@ ipc_socket_create_tcp (DiagnosticsIpc *ipc)
        new_socket = socket (ipc->server_address_family, socket_type, IPPROTO_TCP);
        if (new_socket != DS_IPC_INVALID_SOCKET) {
 #ifndef SOCK_CLOEXEC
-               if (fcntl (new_socket, F_SETFD, FD_CLOEXEC) == -1) {
-                       EP_ASSERT (!"Failed to set CLOEXEC");
-               }
+#ifndef HOST_WIN32
+               fcntl (new_socket, F_SETFD, FD_CLOEXEC); // ignore any failures; this is best effort
+#endif // HOST_WIN32
 #endif // SOCK_CLOEXEC
                int option_value = 1;
                setsockopt (new_socket, IPPROTO_TCP, TCP_NODELAY, (const char*)&option_value, sizeof (option_value));