From 3a9da3a15aad2e898d564a0b6d93fab780295544 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Sat, 5 Jul 2025 22:18:05 +1000 Subject: [PATCH 1/6] Use `$NOCANCEL` variants of APIs on apple platforms See https://github.com/dotnet/runtime/issues/117299 --- eng/native/configurecompiler.cmake | 2 ++ 1 file changed, 2 insertions(+) diff --git a/eng/native/configurecompiler.cmake b/eng/native/configurecompiler.cmake index e8ad8615f6d4bd..2957221d466617 100644 --- a/eng/native/configurecompiler.cmake +++ b/eng/native/configurecompiler.cmake @@ -308,6 +308,8 @@ elseif(CLR_CMAKE_HOST_APPLE) add_definitions(-D_XOPEN_SOURCE) # enable support for Darwin extension APIs, like pthread_getthreadid_np add_definitions(-D_DARWIN_C_SOURCE) + # enable the non-cancellable versions of APIs with $NOCANCEL variants, like close(2) + add_definitions(-D__DARWIN_NON_CANCELABLE=1) if(CLR_CMAKE_HOST_OSX) # the new linker in Xcode 15 (ld_new/ld_prime) deprecated the -bind_at_load flag for macOS which causes a warning From fe1a63077023b688422d2a79a9aa445a1364234e Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Sun, 6 Jul 2025 11:02:59 +1000 Subject: [PATCH 2/6] Remove remaining retry logic for `close` calls - 2 cases still had retry on `EINTR` logic for `close` --- src/coreclr/pal/src/sharedmemory/sharedmemory.cpp | 7 +------ src/native/eventpipe/ds-ipc-pal-socket.c | 4 +--- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/coreclr/pal/src/sharedmemory/sharedmemory.cpp b/src/coreclr/pal/src/sharedmemory/sharedmemory.cpp index 5d12b850f9b9bc..1ec85658594d5f 100644 --- a/src/coreclr/pal/src/sharedmemory/sharedmemory.cpp +++ b/src/coreclr/pal/src/sharedmemory/sharedmemory.cpp @@ -582,12 +582,7 @@ int SharedMemoryHelpers::CreateOrOpenFile( void SharedMemoryHelpers::CloseFile(int fileDescriptor) { _ASSERTE(fileDescriptor != -1); - - int closeResult; - do - { - closeResult = close(fileDescriptor); - } while (closeResult != 0 && errno == EINTR); + close(fileDescriptor); } int SharedMemoryHelpers::ChangeMode(LPCSTR path, mode_t mode) diff --git a/src/native/eventpipe/ds-ipc-pal-socket.c b/src/native/eventpipe/ds-ipc-pal-socket.c index 4a16fb5f81f253..89df116e8f6437 100644 --- a/src/native/eventpipe/ds-ipc-pal-socket.c +++ b/src/native/eventpipe/ds-ipc-pal-socket.c @@ -420,9 +420,7 @@ ipc_socket_close (ds_ipc_socket_t s) #ifdef HOST_WIN32 result_close = closesocket (s); #else - do { - result_close = close (s); - } while (ipc_retry_syscall (result_close)); + result_close = close (s); #endif DS_EXIT_BLOCKING_PAL_SECTION; return result_close; From 40bccbd965178dc2b9e8a0920e4b9c64f1d2664d Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Sun, 6 Jul 2025 11:11:17 +1000 Subject: [PATCH 3/6] Remove dead code in `SafeFileHandle.Unix.cs` --- .../Win32/SafeHandles/SafeFileHandle.Unix.cs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs index 476af77370bd66..003e5926c44d9e 100644 --- a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs @@ -123,10 +123,6 @@ private static SafeFileHandle Open(string path, Interop.Sys.OpenFlags flags, int return handle; } - // Each thread will have its own copy. This prevents race conditions if the handle had the last error. - [ThreadStatic] - internal static Interop.ErrorInfo? t_lastCloseErrorInfo; - protected override bool ReleaseHandle() { // If DeleteOnClose was requested when constructed, delete the file now. @@ -155,12 +151,7 @@ protected override bool ReleaseHandle() // Close the descriptor. Although close is documented to potentially fail with EINTR, we never want // to retry, as the descriptor could actually have been closed, been subsequently reassigned, and // be in use elsewhere in the process. Instead, we simply check whether the call was successful. - int result = Interop.Sys.Close(handle); - if (result != 0) - { - t_lastCloseErrorInfo = Interop.Sys.GetLastErrorInfo(); - } - return result == 0; + return Interop.Sys.Close(handle) == 0; } public override bool IsInvalid From eeb54649b72e707b98a1aa9ae611b63654f3a7f5 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Sun, 6 Jul 2025 13:06:27 +1000 Subject: [PATCH 4/6] Ensure our handling of close giving EINTR is fully consistent --- src/coreclr/pal/src/misc/perfjitdump.cpp | 3 ++- src/native/libs/System.IO.Ports.Native/pal_serial.c | 4 +++- src/native/libs/System.Native/pal_io.c | 4 +++- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/coreclr/pal/src/misc/perfjitdump.cpp b/src/coreclr/pal/src/misc/perfjitdump.cpp index b9afb95a574052..84680cc2b4ffed 100644 --- a/src/coreclr/pal/src/misc/perfjitdump.cpp +++ b/src/coreclr/pal/src/misc/perfjitdump.cpp @@ -14,6 +14,7 @@ #ifdef JITDUMP_SUPPORTED #include +#include #include #include #include @@ -355,7 +356,7 @@ struct PerfJitDumpState result = close(fd); - if (result == -1) + if (result == -1 && errno != EINTR) return FatalError(); fd = -1; diff --git a/src/native/libs/System.IO.Ports.Native/pal_serial.c b/src/native/libs/System.IO.Ports.Native/pal_serial.c index f80d8655727239..221d12d041bbd0 100644 --- a/src/native/libs/System.IO.Ports.Native/pal_serial.c +++ b/src/native/libs/System.IO.Ports.Native/pal_serial.c @@ -44,7 +44,9 @@ int SystemIoPortsNative_SerialPortClose(intptr_t handle) // ignoring the error - best effort ioctl(fd, TIOCNXCL); - return close(fd); + int result = close(fd); + if (result < 0 && errno == EINTR) result = 0; // on all supported platforms, close(2) returning EINTR still means it was released + return result; } int32_t SystemIoPortsNative_Read(intptr_t fd, void* buffer, int32_t bufferSize) diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index 74bead7dc0ada9..23b226d74e0362 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -343,7 +343,9 @@ intptr_t SystemNative_Open(const char* path, int32_t flags, int32_t mode) int32_t SystemNative_Close(intptr_t fd) { - return close(ToFileDescriptor(fd)); + int result = close(ToFileDescriptor(fd)); + if (result < 0 && errno == EINTR) result = 0; // on all supported platforms, close(2) returning EINTR still means it was released + return result; } intptr_t SystemNative_Dup(intptr_t oldfd) From 29b218716c7a1c7230430f06f692055248430a47 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Sun, 6 Jul 2025 13:11:32 +1000 Subject: [PATCH 5/6] Remove a now out-of-date comment --- .../src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs index 003e5926c44d9e..7c92c4e081849a 100644 --- a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs @@ -148,9 +148,7 @@ protected override bool ReleaseHandle() _isLocked = false; } - // Close the descriptor. Although close is documented to potentially fail with EINTR, we never want - // to retry, as the descriptor could actually have been closed, been subsequently reassigned, and - // be in use elsewhere in the process. Instead, we simply check whether the call was successful. + // Close the descriptor. return Interop.Sys.Close(handle) == 0; } From bde273faaac2321a6b6bbea063ccaf2be4e187a9 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Mon, 7 Jul 2025 16:12:22 +1000 Subject: [PATCH 6/6] Update sgen-protocol.c Fix 1 remaining case of `close` being called in a loop --- src/mono/mono/sgen/sgen-protocol.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/mono/mono/sgen/sgen-protocol.c b/src/mono/mono/sgen/sgen-protocol.c index 7c7543cd4292c8..7cfca07f14da8b 100644 --- a/src/mono/mono/sgen/sgen-protocol.c +++ b/src/mono/mono/sgen/sgen-protocol.c @@ -169,8 +169,7 @@ close_binary_protocol_file (void) #if defined(HOST_WIN32) CloseHandle (binary_protocol_file); #elif defined(HAVE_UNISTD_H) - while (close (binary_protocol_file) == -1 && errno == EINTR) - ; + close (binary_protocol_file); #endif binary_protocol_file = invalid_file_value; }