Skip to content

Commit 8dfc3ad

Browse files
hamarb123jkotas
andauthored
Use $NOCANCEL variants of APIs on apple platforms (#117330)
* Use `$NOCANCEL` variants of APIs on apple platforms See #117299 * Remove remaining retry logic for `close` calls - 2 cases still had retry on `EINTR` logic for `close` * Remove dead code in `SafeFileHandle.Unix.cs` * Ensure our handling of close giving EINTR is fully consistent * Remove a now out-of-date comment * Update sgen-protocol.c Fix 1 remaining case of `close` being called in a loop --------- Co-authored-by: Jan Kotas <jkotas@microsoft.com>
1 parent 97da14c commit 8dfc3ad

File tree

8 files changed

+15
-27
lines changed

8 files changed

+15
-27
lines changed

eng/native/configurecompiler.cmake

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,8 @@ elseif(CLR_CMAKE_HOST_APPLE)
308308
add_definitions(-D_XOPEN_SOURCE)
309309
# enable support for Darwin extension APIs, like pthread_getthreadid_np
310310
add_definitions(-D_DARWIN_C_SOURCE)
311+
# enable the non-cancellable versions of APIs with $NOCANCEL variants, like close(2)
312+
add_definitions(-D__DARWIN_NON_CANCELABLE=1)
311313

312314
if(CLR_CMAKE_HOST_OSX)
313315
# the new linker in Xcode 15 (ld_new/ld_prime) deprecated the -bind_at_load flag for macOS which causes a warning

src/coreclr/pal/src/misc/perfjitdump.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#ifdef JITDUMP_SUPPORTED
1515

1616
#include <fcntl.h>
17+
#include <errno.h>
1718
#include <pthread.h>
1819
#include <stdint.h>
1920
#include <stdio.h>
@@ -355,7 +356,7 @@ struct PerfJitDumpState
355356

356357
result = close(fd);
357358

358-
if (result == -1)
359+
if (result == -1 && errno != EINTR)
359360
return FatalError();
360361

361362
fd = -1;

src/coreclr/pal/src/sharedmemory/sharedmemory.cpp

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -582,12 +582,7 @@ int SharedMemoryHelpers::CreateOrOpenFile(
582582
void SharedMemoryHelpers::CloseFile(int fileDescriptor)
583583
{
584584
_ASSERTE(fileDescriptor != -1);
585-
586-
int closeResult;
587-
do
588-
{
589-
closeResult = close(fileDescriptor);
590-
} while (closeResult != 0 && errno == EINTR);
585+
close(fileDescriptor);
591586
}
592587

593588
int SharedMemoryHelpers::ChangeMode(LPCSTR path, mode_t mode)

src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -123,10 +123,6 @@ private static SafeFileHandle Open(string path, Interop.Sys.OpenFlags flags, int
123123
return handle;
124124
}
125125

126-
// Each thread will have its own copy. This prevents race conditions if the handle had the last error.
127-
[ThreadStatic]
128-
internal static Interop.ErrorInfo? t_lastCloseErrorInfo;
129-
130126
protected override bool ReleaseHandle()
131127
{
132128
// If DeleteOnClose was requested when constructed, delete the file now.
@@ -152,15 +148,8 @@ protected override bool ReleaseHandle()
152148
_isLocked = false;
153149
}
154150

155-
// Close the descriptor. Although close is documented to potentially fail with EINTR, we never want
156-
// to retry, as the descriptor could actually have been closed, been subsequently reassigned, and
157-
// be in use elsewhere in the process. Instead, we simply check whether the call was successful.
158-
int result = Interop.Sys.Close(handle);
159-
if (result != 0)
160-
{
161-
t_lastCloseErrorInfo = Interop.Sys.GetLastErrorInfo();
162-
}
163-
return result == 0;
151+
// Close the descriptor.
152+
return Interop.Sys.Close(handle) == 0;
164153
}
165154

166155
public override bool IsInvalid

src/mono/mono/sgen/sgen-protocol.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,7 @@ close_binary_protocol_file (void)
169169
#if defined(HOST_WIN32)
170170
CloseHandle (binary_protocol_file);
171171
#elif defined(HAVE_UNISTD_H)
172-
while (close (binary_protocol_file) == -1 && errno == EINTR)
173-
;
172+
close (binary_protocol_file);
174173
#endif
175174
binary_protocol_file = invalid_file_value;
176175
}

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -420,9 +420,7 @@ ipc_socket_close (ds_ipc_socket_t s)
420420
#ifdef HOST_WIN32
421421
result_close = closesocket (s);
422422
#else
423-
do {
424-
result_close = close (s);
425-
} while (ipc_retry_syscall (result_close));
423+
result_close = close (s);
426424
#endif
427425
DS_EXIT_BLOCKING_PAL_SECTION;
428426
return result_close;

src/native/libs/System.IO.Ports.Native/pal_serial.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,9 @@ int SystemIoPortsNative_SerialPortClose(intptr_t handle)
4444

4545
// ignoring the error - best effort
4646
ioctl(fd, TIOCNXCL);
47-
return close(fd);
47+
int result = close(fd);
48+
if (result < 0 && errno == EINTR) result = 0; // on all supported platforms, close(2) returning EINTR still means it was released
49+
return result;
4850
}
4951

5052
int32_t SystemIoPortsNative_Read(intptr_t fd, void* buffer, int32_t bufferSize)

src/native/libs/System.Native/pal_io.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,9 @@ intptr_t SystemNative_Open(const char* path, int32_t flags, int32_t mode)
343343

344344
int32_t SystemNative_Close(intptr_t fd)
345345
{
346-
return close(ToFileDescriptor(fd));
346+
int result = close(ToFileDescriptor(fd));
347+
if (result < 0 && errno == EINTR) result = 0; // on all supported platforms, close(2) returning EINTR still means it was released
348+
return result;
347349
}
348350

349351
intptr_t SystemNative_Dup(intptr_t oldfd)

0 commit comments

Comments
 (0)