From c11ecefdd114f620589ccbfc910da10bf0bb8eb1 Mon Sep 17 00:00:00 2001 From: Koundinya Veluri Date: Thu, 21 Sep 2023 10:19:04 -0700 Subject: [PATCH 1/7] Include info about system call errors in some exceptions from operating on named mutexes - Added `BeginTrackingSystemCallErrors` and `EndTrackingSystemCallErrors`, and called them around some named mutex operations - `BeginTrackingSystemCallErrors` starts tracking system call errors in the relevant PAL APIs. When there is a system call failure that leads to the PAL API failing, some info is appeneded to a string held on the thread, including the system call, relevant arguments, return value, and `errno`. - `EndTrackingSystemCallErrors` stops tracking system call errors in the relevant PAL APIs and returns the system call errors accumulated so far. - When throwing an exception that doesn't already indicate clearly the reason for the exception, and there are system call errors to report, it would have an inner exception of type `SystemException` that contains the system call errors - `chmod` on OSX seemingly can be interrupted by signals, fixed to retry. Also fixed a couple other small things. Fixes https://github.com/dotnet/runtime/issues/89090 --- .../InteropServices/Marshal.CoreCLR.cs | 23 ++ .../InteropServices/Marshal.NativeAot.cs | 21 ++ src/coreclr/pal/inc/pal.h | 10 + .../pal/src/include/pal/sharedmemory.h | 8 +- src/coreclr/pal/src/include/pal/thread.hpp | 25 +- src/coreclr/pal/src/include/pal/utils.h | 3 + src/coreclr/pal/src/misc/error.cpp | 67 ++++++ src/coreclr/pal/src/misc/utils.cpp | 58 +++++ .../pal/src/sharedmemory/sharedmemory.cpp | 225 ++++++++++++++++-- src/coreclr/pal/src/synchmgr/wait.cpp | 1 + src/coreclr/pal/src/synchobj/mutex.cpp | 23 ++ src/coreclr/pal/src/thread/thread.cpp | 92 +++++++ src/coreclr/vm/ecalllist.h | 2 + src/coreclr/vm/marshalnative.cpp | 37 +++ src/coreclr/vm/marshalnative.h | 2 + .../src/Resources/Strings.resx | 3 + .../src/System/Threading/Mutex.Windows.cs | 84 ++++++- .../Runtime/InteropServices/Marshal.Mono.cs | 21 ++ 18 files changed, 663 insertions(+), 42 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs index 8e2596f994fd1a..e28898b5279304 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs @@ -206,6 +206,29 @@ private static unsafe void WriteValueSlow(object ptr, int ofs, T val, Action< [MethodImpl(MethodImplOptions.InternalCall)] public static extern void SetLastPInvokeError(int error); + /// + /// Begins tracking system call errors on the calling thread during PAL APIs. The system call errors may include + /// information about the system call made, relevant arguments, return values, and error codes. A call to this method + /// should be followed by a call to on the same thread, which returns the set + /// of system call errors that occurred on the thread in that period. Only system call errors that lead to PAL API + /// failures may be tracked. + /// + [MethodImpl(MethodImplOptions.InternalCall)] + internal static extern void BeginTrackingSystemCallErrors(); + + /// + /// Retrieves system call errors that occurred on the calling thread since + /// was called. + /// + /// Indicates whether to return the accumulated system call errors. + /// + /// System call errors that occurred on the calling thread since was called. + /// Returns null if has not been called, or if no system call + /// errors occurred on the calling thread since it was last called. + /// + [MethodImpl(MethodImplOptions.InternalCall)] + internal static extern string EndTrackingSystemCallErrors(bool getSystemCallErrors); + private static void PrelinkCore(MethodInfo m) { if (!(m is RuntimeMethodInfo rmi)) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.NativeAot.cs index a3c5c5d9e1884b..9a56fd07978b05 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.NativeAot.cs @@ -210,6 +210,27 @@ public static void SetLastPInvokeError(int error) PInvokeMarshal.t_lastError = error; } + /// + /// Begins tracking system call errors on the calling thread during PAL APIs. The system call errors may include + /// information about the system call made, relevant arguments, return values, and error codes. A call to this method + /// should be followed by a call to on the same thread, which returns the set + /// of system call errors that occurred on the thread in that period. Only system call errors that lead to PAL API + /// failures may be tracked. + /// + internal static void BeginTrackingSystemCallErrors() { } + + /// + /// Retrieves system call errors that occurred on the calling thread since + /// was called. + /// + /// Indicates whether to return the accumulated system call errors. + /// + /// System call errors that occurred on the calling thread since was called. + /// Returns null if has not been called, or if no system call + /// errors occurred on the calling thread since it was last called. + /// + internal static string EndTrackingSystemCallErrors(bool getSystemCallErrors) => null; + internal static bool IsPinnable(object o) { return (o == null) || !o.GetEETypePtr().ContainsGCPointers; diff --git a/src/coreclr/pal/inc/pal.h b/src/coreclr/pal/inc/pal.h index 2bf5b17344d3de..19f046395a74ab 100644 --- a/src/coreclr/pal/inc/pal.h +++ b/src/coreclr/pal/inc/pal.h @@ -3792,6 +3792,16 @@ PALAPI SetLastError( IN DWORD dwErrCode); +PALIMPORT +VOID +PALAPI +PAL_BeginTrackingSystemCallErrors(); + +PALIMPORT +LPCSTR +PALAPI +PAL_EndTrackingSystemCallErrors(bool getSystemCallErrors); + PALIMPORT LPWSTR PALAPI diff --git a/src/coreclr/pal/src/include/pal/sharedmemory.h b/src/coreclr/pal/src/include/pal/sharedmemory.h index 88834b93d06738..be01499586a08b 100644 --- a/src/coreclr/pal/src/include/pal/sharedmemory.h +++ b/src/coreclr/pal/src/include/pal/sharedmemory.h @@ -114,10 +114,12 @@ class SharedMemoryHelpers static int CreateOrOpenFile(LPCSTR path, bool createIfNotExist = true, bool *createdRef = nullptr); static void CloseFile(int fileDescriptor); - static SIZE_T GetFileSize(int fileDescriptor); - static void SetFileSize(int fileDescriptor, SIZE_T byteCount); + static int ChangeMode(LPCSTR path, mode_t mode); - static void *MemoryMapFile(int fileDescriptor, SIZE_T byteCount); + static SIZE_T GetFileSize(LPCSTR filePath, int fileDescriptor); + static void SetFileSize(LPCSTR filePath, int fileDescriptor, SIZE_T byteCount); + + static void *MemoryMapFile(LPCSTR filePath, int fileDescriptor, SIZE_T byteCount); static bool TryAcquireFileLock(int fileDescriptor, int operation); static void ReleaseFileLock(int fileDescriptor); diff --git a/src/coreclr/pal/src/include/pal/thread.hpp b/src/coreclr/pal/src/include/pal/thread.hpp index 86850b260c9301..4932f084edbce4 100644 --- a/src/coreclr/pal/src/include/pal/thread.hpp +++ b/src/coreclr/pal/src/include/pal/thread.hpp @@ -304,6 +304,10 @@ namespace CorUnix // Signal handler's alternate stack to help with stack overflow void* m_alternateStack; + bool m_isTrackingSystemCallErrors; + int m_systemCallErrorsLength; + char *m_systemCallErrors; + // // The thread entry routine (called from InternalCreateThread) // @@ -358,7 +362,10 @@ namespace CorUnix m_fStartStatusSet(FALSE), m_stackBase(NULL), m_stackLimit(NULL), - m_alternateStack(NULL) + m_alternateStack(NULL), + m_isTrackingSystemCallErrors(false), + m_systemCallErrorsLength(0), + m_systemCallErrors(NULL) { }; @@ -460,6 +467,22 @@ namespace CorUnix return errno; }; + void + BeginTrackingSystemCallErrors( + void + ); + + static void + AppendSystemCallError( + LPCSTR format, + ... + ); + + LPCSTR + EndTrackingSystemCallErrors( + bool getSystemCallErrors + ); + void SetExitCode( DWORD dwExitCode diff --git a/src/coreclr/pal/src/include/pal/utils.h b/src/coreclr/pal/src/include/pal/utils.h index 83cf2b104c1ff9..2a7ffc3d2ef854 100644 --- a/src/coreclr/pal/src/include/pal/utils.h +++ b/src/coreclr/pal/src/include/pal/utils.h @@ -212,3 +212,6 @@ class StringHolder }; #endif /* _PAL_UTILS_H_ */ + +const int RawErrorCodeStringBufferSize = 12; +const char *GetFriendlyErrorCodeString(int errorCode, char (&rawErrorCodeStringBuffer)[RawErrorCodeStringBufferSize]); diff --git a/src/coreclr/pal/src/misc/error.cpp b/src/coreclr/pal/src/misc/error.cpp index 6b67bf717d52cb..f4a30aa4700aca 100644 --- a/src/coreclr/pal/src/misc/error.cpp +++ b/src/coreclr/pal/src/misc/error.cpp @@ -84,3 +84,70 @@ SetLastError( CPalThread::SetLastError(dwErrCode); } +/*++ +Function: + PAL_BeginTrackingSystemCallErrors + +PAL_BeginTrackingSystemCallErrors + +The PAL_BeginTrackingSystemCallErrors function begins tracking system call errors on the calling thread during PAL APIs. The +system call errors may include information about the system call made, relevant arguments, return values, and error codes. A +call to this function should be followed by a call to PAL_EndTrackingSystemCallErrors on the same thread, which returns the set +of system call errors that occurred on the thread in that period. This may not track all system call errors, it may only track +system call errors that directly or indirectly lead to PAL API failures. + +Parameters + +This function has no parameters. + +Return Values + +This function does not return a value. + +--*/ +VOID +PALAPI +PAL_BeginTrackingSystemCallErrors( + VOID) +{ + CPalThread *thread = GetCurrentPalThread(); + if (thread != nullptr) + { + thread->BeginTrackingSystemCallErrors(); + } +} + +/*++ +Function: + PAL_EndTrackingSystemCallErrors + +PAL_EndTrackingSystemCallErrors + +The PAL_EndTrackingSystemCallErrors function retrieves system call errors that occurred on the calling thread since +PAL_BeginTrackingSystemCallErrors was called. + +Parameters + +This function has no parameters. + +Return Values + +The return value is the system call errors that occurred on the calling thread since PAL_BeginTrackingSystemCallErrors was +called. Returns NULL if PAL_BeginTrackingSystemCallErrors has not been called, or if no system call errors occurred on the +calling thread since it was last called. If the returned pointer is not NULL, it is only safe to be used by the calling thread +and before the next call to PAL_BeginTrackingSystemCallErrors. + +--*/ +LPCSTR +PALAPI +PAL_EndTrackingSystemCallErrors( + bool getSystemCallErrors) +{ + CPalThread *thread = GetCurrentPalThread(); + if (thread != nullptr) + { + return thread->EndTrackingSystemCallErrors(getSystemCallErrors); + } + + return nullptr; +} diff --git a/src/coreclr/pal/src/misc/utils.cpp b/src/coreclr/pal/src/misc/utils.cpp index f279ef3d580c14..2c2bb4f57ab123 100644 --- a/src/coreclr/pal/src/misc/utils.cpp +++ b/src/coreclr/pal/src/misc/utils.cpp @@ -366,3 +366,61 @@ BOOL IsRunningOnMojaveHardenedRuntime() } #endif // __APPLE__ + +const char *GetFriendlyErrorCodeString(int errorCode, char (&rawErrorCodeStringBuffer)[RawErrorCodeStringBufferSize]) +{ + switch (errorCode) + { + case EACCES: return "EACCES"; + case EBADF: return "EBADF"; + case EBUSY: return "EBUSY"; + case EDQUOT: return "EDQUOT"; + case EEXIST: return "EEXIST"; + case EFAULT: return "EFAULT"; + case EFBIG: return "EFBIG"; + case EINVAL: return "EINVAL"; + case EINTR: return "EINTR"; + case EIO: return "EIO"; + case EISDIR: return "EISDIR"; + case ELOOP: return "ELOOP"; + case EMFILE: return "EMFILE"; + case EMLINK: return "EMLINK"; + case ENAMETOOLONG: return "ENAMETOOLONG"; + case ENFILE: return "ENFILE"; + case ENODEV: return "ENODEV"; + case ENOENT: return "ENOENT"; + case ENOLCK: return "ENOLCK"; + case ENOMEM: return "ENOMEM"; + case ENOSPC: return "ENOSPC"; + case ENOTDIR: return "ENOTDIR"; + case ENOTEMPTY: return "ENOTEMPTY"; + case ENXIO: return "ENXIO"; + case EOVERFLOW: return "EOVERFLOW"; + case EPERM: return "EPERM"; + case EROFS: return "EROFS"; + case ETXTBSY: return "ETXTBSY"; + case EXDEV: return "EXDEV"; + } + + if (errorCode == EAGAIN || errorCode == EWOULDBLOCK) + { + if (EAGAIN == EWOULDBLOCK) return "EAGAIN/EWOULDBLOCK"; + if (errorCode == EAGAIN) return "EAGAIN"; + return "EWOULDBLOCK"; + } + else if (errorCode == ENOTSUP || errorCode == EOPNOTSUPP) + { + if (ENOTSUP == EOPNOTSUPP) return "ENOTSUP/EOPNOTSUPP"; + if (errorCode == ENOTSUP) return "ENOTSUP"; + return "EOPNOTSUPP"; + } + + int result = + _snprintf_s(rawErrorCodeStringBuffer, RawErrorCodeStringBufferSize, RawErrorCodeStringBufferSize - 1, "%d", errorCode); + if (result <= 0 || result >= RawErrorCodeStringBufferSize) + { + rawErrorCodeStringBuffer[0] = '\0'; + } + + return rawErrorCodeStringBuffer; +} diff --git a/src/coreclr/pal/src/sharedmemory/sharedmemory.cpp b/src/coreclr/pal/src/sharedmemory/sharedmemory.cpp index a2342f23efa5cf..cc78831ac2240d 100644 --- a/src/coreclr/pal/src/sharedmemory/sharedmemory.cpp +++ b/src/coreclr/pal/src/sharedmemory/sharedmemory.cpp @@ -1,14 +1,17 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +#include "pal/dbgmsg.h" +SET_DEFAULT_DEBUG_CHANNEL(SHMEM); // some headers have code with asserts, so do this first + #include "pal/sharedmemory.h" -#include "pal/dbgmsg.h" #include "pal/file.hpp" #include "pal/malloc.hpp" #include "pal/thread.hpp" #include "pal/virtual.h" #include "pal/process.h" +#include "pal/utils.h" #include #include @@ -23,8 +26,6 @@ using namespace CorUnix; -SET_DEFAULT_DEBUG_CHANNEL(SHMEM); - #include "pal/sharedmemory.inl" //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -104,6 +105,8 @@ bool SharedMemoryHelpers::EnsureDirectoryExists( _ASSERTE(SharedMemoryManager::IsCreationDeletionProcessLockAcquired()); _ASSERTE(!isGlobalLockAcquired || SharedMemoryManager::IsCreationDeletionFileLockAcquired()); + char rawErrorCodeStringBuffer[RawErrorCodeStringBufferSize]; + // Check if the path already exists struct stat statInfo; int statResult = stat(path, &statInfo); @@ -123,15 +126,33 @@ bool SharedMemoryHelpers::EnsureDirectoryExists( if (isGlobalLockAcquired) { - if (mkdir(path, PermissionsMask_AllUsers_ReadWriteExecute) != 0) + int operationResult = mkdir(path, PermissionsMask_AllUsers_ReadWriteExecute); + if (operationResult != 0) { + int errorCode = errno; + CPalThread::AppendSystemCallError( + "mkdir(\"%s\", 0x%x) == %d; errno == %s;", + path, + (int)PermissionsMask_AllUsers_ReadWriteExecute, + operationResult, + GetFriendlyErrorCodeString(errorCode, rawErrorCodeStringBuffer)); throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } - if (chmod(path, PermissionsMask_AllUsers_ReadWriteExecute) != 0) + + operationResult = ChangeMode(path, PermissionsMask_AllUsers_ReadWriteExecute); + if (operationResult != 0) { + int errorCode = errno; + CPalThread::AppendSystemCallError( + "chmod(\"%s\", 0x%x) == %d; errno == %s;", + path, + (int)PermissionsMask_AllUsers_ReadWriteExecute, + operationResult, + GetFriendlyErrorCodeString(errorCode, rawErrorCodeStringBuffer)); rmdir(path); throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } + return true; } @@ -140,13 +161,28 @@ bool SharedMemoryHelpers::EnsureDirectoryExists( if (mkdtemp(tempPath.OpenStringBuffer()) == nullptr) { + int errorCode = errno; + CPalThread::AppendSystemCallError( + "mkdtemp(\"%s\") == nullptr; errno == %s;", + (const char *)tempPath, + GetFriendlyErrorCodeString(errorCode, rawErrorCodeStringBuffer)); throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } - if (chmod(tempPath, PermissionsMask_AllUsers_ReadWriteExecute) != 0) + + int operationResult = ChangeMode(tempPath, PermissionsMask_AllUsers_ReadWriteExecute); + if (operationResult != 0) { + int errorCode = errno; + CPalThread::AppendSystemCallError( + "chmod(\"%s\", 0x%x) == %d; errno == %s;", + (const char *)tempPath, + (int)PermissionsMask_AllUsers_ReadWriteExecute, + operationResult, + GetFriendlyErrorCodeString(errorCode, rawErrorCodeStringBuffer)); rmdir(tempPath); throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } + if (rename(tempPath, path) == 0) { return true; @@ -161,6 +197,24 @@ bool SharedMemoryHelpers::EnsureDirectoryExists( // If the path exists, check that it's a directory if (statResult != 0 || !(statInfo.st_mode & S_IFDIR)) { + if (statResult != 0) + { + int errorCode = errno; + CPalThread::AppendSystemCallError( + "stat(\"%s\", ...) == %d; errno == %s;", + path, + statResult, + GetFriendlyErrorCodeString(errorCode, rawErrorCodeStringBuffer)); + } + else + { + CPalThread::AppendSystemCallError( + "stat(\"%s\", &info) == 0; info.st_mode == 0x%x; (info.st_mode & 0x%x) == 0;", + path, + (int)statInfo.st_mode, + (int)S_IFDIR); + } + throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } @@ -176,6 +230,13 @@ bool SharedMemoryHelpers::EnsureDirectoryExists( { return true; } + + CPalThread::AppendSystemCallError( + "stat(\"%s\", &info) == 0; info.st_mode == 0x%x; (info.st_mode & 0x%x) != 0x%x;", + path, + (int)statInfo.st_mode, + (int)PermissionsMask_CurrentUser_ReadWriteExecute, + (int)PermissionsMask_CurrentUser_ReadWriteExecute); throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } @@ -186,12 +247,18 @@ bool SharedMemoryHelpers::EnsureDirectoryExists( { return true; } - if (!createIfNotExist || chmod(path, PermissionsMask_AllUsers_ReadWriteExecute) != 0) + if (!createIfNotExist || ChangeMode(path, PermissionsMask_AllUsers_ReadWriteExecute) != 0) { // We were not asked to create the path or we weren't able to set the new permissions. // As a last resort, check that at least the current user has full access. if ((statInfo.st_mode & PermissionsMask_CurrentUser_ReadWriteExecute) != PermissionsMask_CurrentUser_ReadWriteExecute) { + CPalThread::AppendSystemCallError( + "stat(\"%s\", &info) == 0; info.st_mode == 0x%x; (info.st_mode & 0x%x) != 0x%x;", + path, + (int)statInfo.st_mode, + (int)PermissionsMask_CurrentUser_ReadWriteExecute, + (int)PermissionsMask_CurrentUser_ReadWriteExecute); throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } } @@ -213,6 +280,7 @@ int SharedMemoryHelpers::Open(LPCSTR path, int flags, mode_t mode) openErrorCode = errno; } while (openErrorCode == EINTR); + SharedMemoryError sharedMemoryError; switch (openErrorCode) { case ENOENT: @@ -221,16 +289,32 @@ int SharedMemoryHelpers::Open(LPCSTR path, int flags, mode_t mode) return -1; case ENAMETOOLONG: - throw SharedMemoryException(static_cast(SharedMemoryError::NameTooLong)); + sharedMemoryError = SharedMemoryError::NameTooLong; + break; case EMFILE: case ENFILE: case ENOMEM: - throw SharedMemoryException(static_cast(SharedMemoryError::OutOfMemory)); + sharedMemoryError = SharedMemoryError::OutOfMemory; + break; default: - throw SharedMemoryException(static_cast(SharedMemoryError::IO)); + sharedMemoryError = SharedMemoryError::IO; + break; + } + + if (sharedMemoryError != SharedMemoryError::NameTooLong) + { + char rawErrorCodeStringBuffer[RawErrorCodeStringBufferSize]; + CPalThread::AppendSystemCallError( + "open(\"%s\", 0x%x, 0x%x) == -1; errno == %s;", + path, + flags, + (int)mode, + GetFriendlyErrorCodeString(openErrorCode, rawErrorCodeStringBuffer)); } + + throw SharedMemoryException(static_cast(sharedMemoryError)); } int SharedMemoryHelpers::OpenDirectory(LPCSTR path) @@ -278,8 +362,17 @@ int SharedMemoryHelpers::CreateOrOpenFile(LPCSTR path, bool createIfNotExist, bo // The permissions mask passed to open() is filtered by the process' permissions umask, so open() may not set all of // the requested permissions. Use chmod() to set the proper permissions. - if (chmod(path, PermissionsMask_AllUsers_ReadWrite) != 0) + int operationResult = ChangeMode(path, PermissionsMask_AllUsers_ReadWrite); + if (operationResult != 0) { + int errorCode = errno; + char rawErrorCodeStringBuffer[RawErrorCodeStringBufferSize]; + CPalThread::AppendSystemCallError( + "chmod(\"%s\", 0x%x) == %d; errno == %s;", + path, + (int)PermissionsMask_AllUsers_ReadWrite, + operationResult, + GetFriendlyErrorCodeString(errorCode, rawErrorCodeStringBuffer)); CloseFile(fileDescriptor); unlink(path); throw SharedMemoryException(static_cast(SharedMemoryError::IO)); @@ -303,21 +396,47 @@ void SharedMemoryHelpers::CloseFile(int fileDescriptor) } while (closeResult != 0 && errno == EINTR); } -SIZE_T SharedMemoryHelpers::GetFileSize(int fileDescriptor) +int SharedMemoryHelpers::ChangeMode(LPCSTR path, mode_t mode) { + _ASSERTE(path != nullptr); + _ASSERTE(path[0] != '\0'); + + int chmodResult; + do + { + chmodResult = chmod(path, mode); + } while (chmodResult != 0 && errno == EINTR); + + return chmodResult; +} + +SIZE_T SharedMemoryHelpers::GetFileSize(LPCSTR filePath, int fileDescriptor) +{ + _ASSERTE(filePath != nullptr); + _ASSERTE(filePath[0] != '\0'); _ASSERTE(fileDescriptor != -1); off_t endOffset = lseek(fileDescriptor, 0, SEEK_END); if (endOffset == static_cast(-1) || lseek(fileDescriptor, 0, SEEK_SET) == static_cast(-1)) { + int errorCode = errno; + char rawErrorCodeStringBuffer[RawErrorCodeStringBufferSize]; + CPalThread::AppendSystemCallError( + "lseek(\"%s\", 0, %s) == -1; errno == %s;", + filePath, + (int)PermissionsMask_AllUsers_ReadWrite, + GetFriendlyErrorCodeString(errorCode, rawErrorCodeStringBuffer)); throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } + return endOffset; } -void SharedMemoryHelpers::SetFileSize(int fileDescriptor, SIZE_T byteCount) +void SharedMemoryHelpers::SetFileSize(LPCSTR filePath, int fileDescriptor, SIZE_T byteCount) { + _ASSERTE(filePath != nullptr); + _ASSERTE(filePath[0] != '\0'); _ASSERTE(fileDescriptor != -1); _ASSERTE(static_cast(byteCount) == byteCount); @@ -328,15 +447,26 @@ void SharedMemoryHelpers::SetFileSize(int fileDescriptor, SIZE_T byteCount) { break; } - if (errno != EINTR) + + int errorCode = errno; + if (errorCode != EINTR) { + char rawErrorCodeStringBuffer[RawErrorCodeStringBufferSize]; + CPalThread::AppendSystemCallError( + "ftruncate(\"%s\", %zu) == %d; errno == %s;", + filePath, + byteCount, + ftruncateResult, + GetFriendlyErrorCodeString(errorCode, rawErrorCodeStringBuffer)); throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } } } -void *SharedMemoryHelpers::MemoryMapFile(int fileDescriptor, SIZE_T byteCount) +void *SharedMemoryHelpers::MemoryMapFile(LPCSTR filePath, int fileDescriptor, SIZE_T byteCount) { + _ASSERTE(filePath != nullptr); + _ASSERTE(filePath[0] != '\0'); _ASSERTE(fileDescriptor != -1); _ASSERTE(byteCount > sizeof(SharedMemorySharedDataHeader)); _ASSERTE(AlignDown(byteCount, GetVirtualPageSize()) == byteCount); @@ -346,15 +476,29 @@ void *SharedMemoryHelpers::MemoryMapFile(int fileDescriptor, SIZE_T byteCount) { return sharedMemoryBuffer; } - switch (errno) + + int errorCode = errno; + SharedMemoryError sharedMemoryError; + switch (errorCode) { + case EMFILE: case ENFILE: case ENOMEM: - throw SharedMemoryException(static_cast(SharedMemoryError::OutOfMemory)); + sharedMemoryError = SharedMemoryError::OutOfMemory; + break; default: - throw SharedMemoryException(static_cast(SharedMemoryError::IO)); + sharedMemoryError = SharedMemoryError::IO; + break; } + + char rawErrorCodeStringBuffer[RawErrorCodeStringBufferSize]; + CPalThread::AppendSystemCallError( + "mmap(nullptr, %zu, PROT_READ | PROT_WRITE, MAP_SHARED, \"%s\", 0) == MAP_FAILED; errno == %s;", + byteCount, + filePath, + GetFriendlyErrorCodeString(errorCode, rawErrorCodeStringBuffer)); + throw SharedMemoryException(static_cast(sharedMemoryError)); } bool SharedMemoryHelpers::TryAcquireFileLock(int fileDescriptor, int operation) @@ -362,16 +506,19 @@ bool SharedMemoryHelpers::TryAcquireFileLock(int fileDescriptor, int operation) // A file lock is acquired once per file descriptor, so the caller will need to synchronize threads of this process _ASSERTE(fileDescriptor != -1); + _ASSERTE((operation & LOCK_EX) ^ (operation & LOCK_SH)); _ASSERTE(!(operation & LOCK_UN)); while (true) { - if (flock(fileDescriptor, operation) == 0) + int flockResult = flock(fileDescriptor, operation); + if (flockResult == 0) { return true; } int flockError = errno; + SharedMemoryError sharedMemoryError = SharedMemoryError::IO; switch (flockError) { case EWOULDBLOCK: @@ -380,9 +527,20 @@ bool SharedMemoryHelpers::TryAcquireFileLock(int fileDescriptor, int operation) case EINTR: continue; - default: - throw SharedMemoryException(static_cast(SharedMemoryError::OutOfMemory)); + case ENOLCK: + sharedMemoryError = SharedMemoryError::OutOfMemory; + break; } + + char rawErrorCodeStringBuffer[RawErrorCodeStringBufferSize]; + CPalThread::AppendSystemCallError( + "flock(%d, %s%s) == %d; errno == %s;", + fileDescriptor, + operation & LOCK_EX ? "LOCK_EX" : "LOCK_SH", + operation & LOCK_NB ? " | LOCK_NB" : "", + flockResult, + GetFriendlyErrorCodeString(flockError, rawErrorCodeStringBuffer)); + throw SharedMemoryException(static_cast(sharedMemoryError)); } } @@ -711,18 +869,18 @@ SharedMemoryProcessDataHeader *SharedMemoryProcessDataHeader::CreateOrOpen( SIZE_T sharedDataTotalByteCount = SharedMemorySharedDataHeader::GetTotalByteCount(sharedDataByteCount); if (createdFile) { - SharedMemoryHelpers::SetFileSize(fileDescriptor, sharedDataTotalByteCount); + SharedMemoryHelpers::SetFileSize(filePath, fileDescriptor, sharedDataTotalByteCount); } else { - SIZE_T currentFileSize = SharedMemoryHelpers::GetFileSize(fileDescriptor); + SIZE_T currentFileSize = SharedMemoryHelpers::GetFileSize(filePath, fileDescriptor); if (currentFileSize < sharedDataUsedByteCount) { throw SharedMemoryException(static_cast(SharedMemoryError::HeaderMismatch)); } if (currentFileSize < sharedDataTotalByteCount) { - SharedMemoryHelpers::SetFileSize(fileDescriptor, sharedDataTotalByteCount); + SharedMemoryHelpers::SetFileSize(filePath, fileDescriptor, sharedDataTotalByteCount); } } @@ -732,12 +890,18 @@ SharedMemoryProcessDataHeader *SharedMemoryProcessDataHeader::CreateOrOpen( // non-blocking file lock should succeed. if (!SharedMemoryHelpers::TryAcquireFileLock(fileDescriptor, LOCK_SH | LOCK_NB)) { + int errorCode = errno; + char rawErrorCodeStringBuffer[RawErrorCodeStringBufferSize]; + CPalThread::AppendSystemCallError( + "flock(\"%s\", LOCK_SH | LOCK_NB) == -1; errno == %s;", + (const char *)filePath, + GetFriendlyErrorCodeString(errorCode, rawErrorCodeStringBuffer)); throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } autoCleanup.m_acquiredFileLock = true; // Map the file into memory, and initialize or validate the header - void *mappedBuffer = SharedMemoryHelpers::MemoryMapFile(fileDescriptor, sharedDataTotalByteCount); + void *mappedBuffer = SharedMemoryHelpers::MemoryMapFile(filePath, fileDescriptor, sharedDataTotalByteCount); autoCleanup.m_mappedBuffer = mappedBuffer; autoCleanup.m_mappedBufferByteCount = sharedDataTotalByteCount; SharedMemorySharedDataHeader *sharedDataHeader; @@ -1155,17 +1319,28 @@ void SharedMemoryManager::AcquireCreationDeletionFileLock() false /* createIfNotExist */, true /* isSystemDirectory */)) { + _ASSERTE(errno == ENOENT); + CPalThread::AppendSystemCallError("stat(\"%s\", ...) == -1; errno == ENOENT;", (const char *)*gSharedFilesPath); throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } + SharedMemoryHelpers::EnsureDirectoryExists( *s_runtimeTempDirectoryPath, false /* isGlobalLockAcquired */); + SharedMemoryHelpers::EnsureDirectoryExists( *s_sharedMemoryDirectoryPath, false /* isGlobalLockAcquired */); + s_creationDeletionLockFileDescriptor = SharedMemoryHelpers::OpenDirectory(*s_sharedMemoryDirectoryPath); if (s_creationDeletionLockFileDescriptor == -1) { + int errorCode = errno; + char rawErrorCodeStringBuffer[RawErrorCodeStringBufferSize]; + CPalThread::AppendSystemCallError( + "open(\"%s\", O_RDONLY | O_CLOEXEC, 0) == -1; errno == %s;", + (const char *)*s_sharedMemoryDirectoryPath, + GetFriendlyErrorCodeString(errorCode, rawErrorCodeStringBuffer)); throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } } diff --git a/src/coreclr/pal/src/synchmgr/wait.cpp b/src/coreclr/pal/src/synchmgr/wait.cpp index d666d5101ba7ad..e4bab9a6fb74c0 100644 --- a/src/coreclr/pal/src/synchmgr/wait.cpp +++ b/src/coreclr/pal/src/synchmgr/wait.cpp @@ -229,6 +229,7 @@ SignalObjectAndWait( bAlertable ? "TRUE" : "FALSE"); CPalThread *thread = InternalGetCurrentThread(); + DWORD result = InternalSignalObjectAndWait(thread, hObjectToSignal, hObjectToWaitOn, dwMilliseconds, bAlertable); LOGEXIT("SignalObjectAndWait returns DWORD %u\n", result); diff --git a/src/coreclr/pal/src/synchobj/mutex.cpp b/src/coreclr/pal/src/synchobj/mutex.cpp index 3fff2e7917fade..0da0fa8f73b50c 100644 --- a/src/coreclr/pal/src/synchobj/mutex.cpp +++ b/src/coreclr/pal/src/synchobj/mutex.cpp @@ -23,6 +23,7 @@ SET_DEFAULT_DEBUG_CHANNEL(SYNC); // some headers have code with asserts, so do t #include "pal/mutex.hpp" #include "pal/file.hpp" #include "pal/thread.hpp" +#include "pal/utils.h" #include "../synchmgr/synchmanager.hpp" @@ -750,6 +751,8 @@ void MutexHelpers::InitializeProcessSharedRobustRecursiveMutex(pthread_mutex_t * { _ASSERTE(mutex != nullptr); + char rawErrorCodeStringBuffer[RawErrorCodeStringBufferSize]; + struct AutoCleanup { pthread_mutexattr_t *m_mutexAttributes; @@ -772,6 +775,9 @@ void MutexHelpers::InitializeProcessSharedRobustRecursiveMutex(pthread_mutex_t * int error = pthread_mutexattr_init(&mutexAttributes); if (error != 0) { + CPalThread::AppendSystemCallError( + "pthread_mutexattr_init(...) == %d;", + GetFriendlyErrorCodeString(error, rawErrorCodeStringBuffer)); throw SharedMemoryException(static_cast(SharedMemoryError::OutOfMemory)); } autoCleanup.m_mutexAttributes = &mutexAttributes; @@ -788,6 +794,9 @@ void MutexHelpers::InitializeProcessSharedRobustRecursiveMutex(pthread_mutex_t * error = pthread_mutex_init(mutex, &mutexAttributes); if (error != 0) { + CPalThread::AppendSystemCallError( + "pthread_mutex_init(...) == %d;", + GetFriendlyErrorCodeString(error, rawErrorCodeStringBuffer)); throw SharedMemoryException(static_cast(error == EPERM ? SharedMemoryError::IO : SharedMemoryError::OutOfMemory)); } } @@ -850,7 +859,16 @@ MutexTryAcquireLockResult MutexHelpers::TryAcquireLock(pthread_mutex_t *mutex, D throw SharedMemoryException(static_cast(NamedMutexError::MaximumRecursiveLocksReached)); default: + { + char rawErrorCodeStringBuffer[RawErrorCodeStringBufferSize]; + CPalThread::AppendSystemCallError( + "%s(...) == %d;", + timeoutMilliseconds == (DWORD)-1 ? "pthread_mutex_lock" + : timeoutMilliseconds == 0 ? "pthread_mutex_trylock" + : "pthread_mutex_timedlock", + GetFriendlyErrorCodeString(lockResult, rawErrorCodeStringBuffer)); throw SharedMemoryException(static_cast(NamedMutexError::Unknown)); + } } } @@ -917,6 +935,7 @@ void NamedMutexSharedData::IncTimedWaiterCount() ULONG newValue = InterlockedIncrement(reinterpret_cast(&m_timedWaiterCount)); if (newValue == 0) { + InterlockedDecrement(reinterpret_cast(&m_timedWaiterCount)); throw SharedMemoryException(static_cast(SharedMemoryError::OutOfMemory)); } } @@ -1138,8 +1157,12 @@ SharedMemoryProcessDataHeader *NamedMutexProcessData::CreateOrOpen( _ASSERTE(!created); if (createIfNotExist) { + CPalThread::AppendSystemCallError( + "open(\"%s\", O_RDWR | O_CREAT | O_EXCL | O_CLOEXEC, 0) == -1; errno == ENOENT;", + (const char *)lockFilePath); throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } + return nullptr; } autoCleanup.m_createdLockFile = created; diff --git a/src/coreclr/pal/src/thread/thread.cpp b/src/coreclr/pal/src/thread/thread.cpp index 90501bbafbde56..a0c54bb3653de6 100644 --- a/src/coreclr/pal/src/thread/thread.cpp +++ b/src/coreclr/pal/src/thread/thread.cpp @@ -2258,6 +2258,8 @@ CPalThread::~CPalThread() iError = pthread_mutex_destroy(&m_startMutex); _ASSERTE(0 == iError); } + + delete m_systemCallErrors; } void @@ -2699,6 +2701,96 @@ CPalThread::GetCachedStackLimit() return m_stackLimit; } +void +CPalThread::BeginTrackingSystemCallErrors() +{ + _ASSERTE(!m_isTrackingSystemCallErrors); + + m_isTrackingSystemCallErrors = true; + m_systemCallErrorsLength = 0; + if (m_systemCallErrors != nullptr) + { + m_systemCallErrors[0] = '\0'; + } +} + +void +CPalThread::AppendSystemCallError( + LPCSTR format, + ... + ) +{ + CPalThread *thread = GetCurrentPalThread(); + if (thread == nullptr || !thread->m_isTrackingSystemCallErrors) + { + return; + } + + const int BufferSize = 256; + char *buffer = thread->m_systemCallErrors; + if (buffer == nullptr) + { + buffer = new(std::nothrow) char[BufferSize]; + if (buffer == nullptr) + { + return; + } + + buffer[0] = '\0'; + thread->m_systemCallErrors = buffer; + } + + int length = thread->m_systemCallErrorsLength; + _ASSERTE(length < BufferSize); + _ASSERTE(buffer[length] == '\0'); + if (length >= BufferSize - 1) + { + return; + } + + if (length != 0) + { + length++; // the previous null terminator will be changed to a space if the append succeeds + } + + va_list args; + va_start(args, format); + int result = _vsnprintf_s(buffer + length, BufferSize - length, BufferSize - 1 - length, format, args); + va_end(args); + + if (result == 0) + { + return; + } + + if (result < 0 || result >= BufferSize - length) + { + // There's not enough space to append this error, discard the append and stop tracking system call errors + if (length == 0) + { + buffer[0] = '\0'; + } + thread->m_isTrackingSystemCallErrors = false; + return; + } + + if (length != 0) + { + buffer[length - 1] = ' '; // change the previous null terminator to a space + } + + length += result; + _ASSERTE(thread->m_systemCallErrors[length] == '\0'); + thread->m_systemCallErrorsLength = length; +} + +LPCSTR +CPalThread::EndTrackingSystemCallErrors(bool getSystemCallErrors) +{ + m_isTrackingSystemCallErrors = false; + return getSystemCallErrors ? m_systemCallErrors : nullptr; +} + PVOID PALAPI PAL_GetStackBase() diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index c8c83502cfbab2..1b2f1a2fec24bd 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/src/coreclr/vm/ecalllist.h @@ -474,6 +474,8 @@ FCFuncEnd() FCFuncStart(gInteropMarshalFuncs) FCFuncElement("GetLastPInvokeError", MarshalNative::GetLastPInvokeError) FCFuncElement("SetLastPInvokeError", MarshalNative::SetLastPInvokeError) + FCFuncElement("BeginTrackingSystemCallErrors", MarshalNative::BeginTrackingSystemCallErrors) + FCFuncElement("EndTrackingSystemCallErrors", MarshalNative::EndTrackingSystemCallErrors) FCFuncElement("SizeOfHelper", MarshalNative::SizeOfClass) FCFuncElement("StructureToPtr", MarshalNative::StructureToPtr) FCFuncElement("PtrToStructureHelper", MarshalNative::PtrToStructureHelper) diff --git a/src/coreclr/vm/marshalnative.cpp b/src/coreclr/vm/marshalnative.cpp index 90ea48c1796e5f..b936113494dd39 100644 --- a/src/coreclr/vm/marshalnative.cpp +++ b/src/coreclr/vm/marshalnative.cpp @@ -446,6 +446,43 @@ FCIMPL1(void, MarshalNative::SetLastPInvokeError, int error) } FCIMPLEND +/************************************************************************ + * Marshal.BeginTrackingSystemCallErrors + */ +FCIMPL0(void, MarshalNative::BeginTrackingSystemCallErrors) +{ + FCALL_CONTRACT; + +#ifdef TARGET_UNIX + PAL_BeginTrackingSystemCallErrors(); +#endif // TARGET_UNIX +} +FCIMPLEND + +/************************************************************************ + * Marshal.EndTrackingSystemCallErrors + */ +FCIMPL1(StringObject *, MarshalNative::EndTrackingSystemCallErrors, CLR_BOOL getSystemCallErrors) +{ + FCALL_CONTRACT; + +#ifdef TARGET_UNIX + STRINGREF errorsRef = NULL; + LPCUTF8 errorsBuffer = PAL_EndTrackingSystemCallErrors(getSystemCallErrors); + if (errorsBuffer != NULL) + { + HELPER_METHOD_FRAME_BEGIN_RET_0(); + errorsRef = StringObject::NewString(errorsBuffer); + HELPER_METHOD_FRAME_END(); + } + + return (StringObject *)OBJECTREFToObject(errorsRef); +#else // !TARGET_UNIX + return NULL; +#endif // TARGET_UNIX +} +FCIMPLEND + /************************************************************************ * Support for the GCHandle class. */ diff --git a/src/coreclr/vm/marshalnative.h b/src/coreclr/vm/marshalnative.h index cb26bf2733fb39..e5615f0c30ea77 100644 --- a/src/coreclr/vm/marshalnative.h +++ b/src/coreclr/vm/marshalnative.h @@ -33,6 +33,8 @@ class MarshalNative static FCDECL1(UINT32, OffsetOfHelper, ReflectFieldObject* pFieldUNSAFE); static FCDECL0(int, GetLastPInvokeError); static FCDECL1(void, SetLastPInvokeError, int error); + static FCDECL0(void, BeginTrackingSystemCallErrors); + static FCDECL1(StringObject *, EndTrackingSystemCallErrors, CLR_BOOL getSystemCallErrors); static FCDECL3(VOID, StructureToPtr, Object* pObjUNSAFE, LPVOID ptr, CLR_BOOL fDeleteOld); static FCDECL3(VOID, PtrToStructureHelper, LPVOID ptr, Object* pObjIn, CLR_BOOL allowValueClasses); diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx index 2e00d445ee47ef..23a0edb40102e2 100644 --- a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx +++ b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx @@ -3461,6 +3461,9 @@ in {0}:line {1} + + One or more system calls failed. Details: {0} + Method cannot be invoked. diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Mutex.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Mutex.Windows.cs index 10a91926c3e02b..3d86556a66edf8 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Mutex.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Mutex.Windows.cs @@ -4,6 +4,7 @@ using System.IO; using Microsoft.Win32.SafeHandles; using System.Runtime.InteropServices; +using System.Diagnostics; namespace System.Threading { @@ -18,8 +19,20 @@ public sealed partial class Mutex : WaitHandle private void CreateMutexCore(bool initiallyOwned, string? name, out bool createdNew) { uint mutexFlags = initiallyOwned ? Interop.Kernel32.CREATE_MUTEX_INITIAL_OWNER : 0; - SafeWaitHandle mutexHandle = Interop.Kernel32.CreateMutexEx(IntPtr.Zero, name, mutexFlags, AccessRights); - int errorCode = Marshal.GetLastPInvokeError(); + SafeWaitHandle mutexHandle = null; + int errorCode; + string? systemCallErrors; + Marshal.BeginTrackingSystemCallErrors(); + try + { + mutexHandle = Interop.Kernel32.CreateMutexEx(IntPtr.Zero, name, mutexFlags, AccessRights); + errorCode = Marshal.GetLastPInvokeError(); + } + finally + { + systemCallErrors = + Marshal.EndTrackingSystemCallErrors(getSystemCallErrors: mutexHandle != null && mutexHandle.IsInvalid); + } if (mutexHandle.IsInvalid) { @@ -32,7 +45,7 @@ private void CreateMutexCore(bool initiallyOwned, string? name, out bool created if (errorCode == Interop.Errors.ERROR_INVALID_HANDLE) throw new WaitHandleCannotBeOpenedException(SR.Format(SR.Threading_WaitHandleCannotBeOpenedException_InvalidHandle, name)); - throw Win32Marshal.GetExceptionForWin32Error(errorCode, name); + throw GetExceptionForWin32ErrorWithSystemCallError(errorCode, name, systemCallErrors); } createdNew = errorCode != Interop.Errors.ERROR_ALREADY_EXISTS; @@ -44,16 +57,30 @@ private static OpenExistingResult OpenExistingWorker(string name, out Mutex? res ArgumentException.ThrowIfNullOrEmpty(name); result = null; - // To allow users to view & edit the ACL's, call OpenMutex - // with parameters to allow us to view & edit the ACL. This will - // fail if we don't have permission to view or edit the ACL's. - // If that happens, ask for less permissions. - SafeWaitHandle myHandle = Interop.Kernel32.OpenMutex(AccessRights, false, name); + SafeWaitHandle myHandle = null; + int errorCode = 0; + string? systemCallErrors; + Marshal.BeginTrackingSystemCallErrors(); + try + { + // To allow users to view & edit the ACL's, call OpenMutex + // with parameters to allow us to view & edit the ACL. This will + // fail if we don't have permission to view or edit the ACL's. + // If that happens, ask for less permissions. + myHandle = Interop.Kernel32.OpenMutex(AccessRights, false, name); + if (myHandle.IsInvalid) + { + errorCode = Marshal.GetLastPInvokeError(); + } + } + finally + { + systemCallErrors = + Marshal.EndTrackingSystemCallErrors(getSystemCallErrors: myHandle != null && myHandle.IsInvalid); + } if (myHandle.IsInvalid) { - int errorCode = Marshal.GetLastPInvokeError(); - myHandle.Dispose(); #if TARGET_UNIX || TARGET_BROWSER || TARGET_WASI @@ -70,7 +97,7 @@ private static OpenExistingResult OpenExistingWorker(string name, out Mutex? res if (Interop.Errors.ERROR_INVALID_HANDLE == errorCode) return OpenExistingResult.NameInvalid; - throw Win32Marshal.GetExceptionForWin32Error(errorCode, name); + throw GetExceptionForWin32ErrorWithSystemCallError(errorCode, name, systemCallErrors); } result = new Mutex(myHandle); @@ -82,10 +109,41 @@ private static OpenExistingResult OpenExistingWorker(string name, out Mutex? res // in a Mutex's ACL is MUTEX_ALL_ACCESS (0x1F0001). public void ReleaseMutex() { - if (!Interop.Kernel32.ReleaseMutex(SafeWaitHandle)) + bool success = true; + string? systemCallErrors; + Marshal.BeginTrackingSystemCallErrors(); + try { - throw new ApplicationException(SR.Arg_SynchronizationLockException); + success = Interop.Kernel32.ReleaseMutex(SafeWaitHandle); } + finally + { + systemCallErrors = Marshal.EndTrackingSystemCallErrors(getSystemCallErrors: !success); + } + + if (!success) + { + SystemException? innerEx = + string.IsNullOrEmpty(systemCallErrors) ? null : GetInnerExceptionForSystemCallErrors(systemCallErrors); + throw new ApplicationException(SR.Arg_SynchronizationLockException, innerEx); + } + } + + private static SystemException GetInnerExceptionForSystemCallErrors(string systemCallErrors) => + new SystemException(SR.Format(SR.SystemException_SystemCallErrors, systemCallErrors)); + + private static Exception GetExceptionForWin32ErrorWithSystemCallError( + int errorCode, + string? path, + string? systemCallErrors) + { + Exception ex = Win32Marshal.GetExceptionForWin32Error(errorCode, path); + if (string.IsNullOrEmpty(systemCallErrors) || ex.GetType() != typeof(IOException)) + { + return ex; + } + + return new IOException(ex.Message, GetInnerExceptionForSystemCallErrors(systemCallErrors)) { HResult = ex.HResult }; } } } diff --git a/src/mono/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Mono.cs index e2574f415c0de6..75f464f75490c8 100644 --- a/src/mono/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Mono.cs @@ -30,6 +30,27 @@ public partial class Marshal [MethodImplAttribute(MethodImplOptions.InternalCall)] public static extern void SetLastPInvokeError(int error); + /// + /// Begins tracking system call errors on the calling thread during PAL APIs. The system call errors may include + /// information about the system call made, relevant arguments, return values, and error codes. A call to this method + /// should be followed by a call to on the same thread, which returns the set + /// of system call errors that occurred on the thread in that period. Only system call errors that lead to PAL API + /// failures may be tracked. + /// + internal static void BeginTrackingSystemCallErrors() { } + + /// + /// Retrieves system call errors that occurred on the calling thread since + /// was called. + /// + /// Indicates whether to return the accumulated system call errors. + /// + /// System call errors that occurred on the calling thread since was called. + /// Returns null if has not been called, or if no system call + /// errors occurred on the calling thread since it was last called. + /// + internal static string EndTrackingSystemCallErrors(bool getSystemCallErrors) => null; + [RequiresDynamicCode("Marshalling code for the object might not be available. Use the DestroyStructure overload instead.")] [MethodImplAttribute(MethodImplOptions.InternalCall)] [EditorBrowsable(EditorBrowsableState.Never)] From a79ef635f9999813cd8568e5123da5c283d8e7ff Mon Sep 17 00:00:00 2001 From: Koundinya Veluri Date: Mon, 25 Sep 2023 11:32:57 -0700 Subject: [PATCH 2/7] Small adjustment --- src/coreclr/pal/src/thread/thread.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/pal/src/thread/thread.cpp b/src/coreclr/pal/src/thread/thread.cpp index a0c54bb3653de6..eeab89b23fd4f4 100644 --- a/src/coreclr/pal/src/thread/thread.cpp +++ b/src/coreclr/pal/src/thread/thread.cpp @@ -2780,7 +2780,7 @@ CPalThread::AppendSystemCallError( } length += result; - _ASSERTE(thread->m_systemCallErrors[length] == '\0'); + _ASSERTE(buffer[length] == '\0'); thread->m_systemCallErrorsLength = length; } From 5a551b919c3ed31fbf464c93bea2c77093a0c57e Mon Sep 17 00:00:00 2001 From: Koundinya Veluri Date: Wed, 27 Sep 2023 09:22:40 -0700 Subject: [PATCH 3/7] Address feedback --- .../System.Private.CoreLib.csproj | 5 +- .../InteropServices/Marshal.CoreCLR.cs | 23 -- .../System/Threading/Mutex.CoreCLR.Unix.cs | 81 ++++ .../System/Threading/Mutex.CoreCLR.Windows.cs | 50 +++ .../src/System/Threading/Mutex.CoreCLR.cs | 89 ++++ .../InteropServices/Marshal.NativeAot.cs | 21 - src/coreclr/pal/inc/pal.h | 30 +- src/coreclr/pal/src/configure.cmake | 1 + src/coreclr/pal/src/include/pal/mutex.hpp | 16 +- .../pal/src/include/pal/sharedmemory.h | 33 +- src/coreclr/pal/src/include/pal/thread.hpp | 16 - src/coreclr/pal/src/include/pal/utils.h | 3 +- src/coreclr/pal/src/misc/error.cpp | 67 --- src/coreclr/pal/src/misc/utils.cpp | 44 +- .../pal/src/sharedmemory/sharedmemory.cpp | 387 ++++++++++++------ src/coreclr/pal/src/synchmgr/wait.cpp | 2 +- src/coreclr/pal/src/synchobj/mutex.cpp | 245 +++++++---- src/coreclr/pal/src/thread/thread.cpp | 90 ---- src/coreclr/vm/ecalllist.h | 2 - src/coreclr/vm/marshalnative.cpp | 37 -- src/coreclr/vm/marshalnative.h | 2 - src/coreclr/vm/qcallentrypoints.cpp | 2 + .../Common/src/System/IO/Win32Marshal.cs | 21 +- .../src/Resources/Strings.resx | 6 +- .../System.Private.CoreLib.Shared.projitems | 4 +- .../src/System/Threading/Mutex.Windows.cs | 96 +---- .../Runtime/InteropServices/Marshal.Mono.cs | 21 - 27 files changed, 765 insertions(+), 629 deletions(-) create mode 100644 src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Unix.cs create mode 100644 src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Windows.cs create mode 100644 src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.cs diff --git a/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj index 888f3f8b1498e0..527e218edd8be0 100644 --- a/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj +++ b/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj @@ -239,6 +239,7 @@ + @@ -290,12 +291,14 @@ + - Common\Interop\Windows\OleAut32\Interop.VariantClear.cs + + diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs index e28898b5279304..8e2596f994fd1a 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.CoreCLR.cs @@ -206,29 +206,6 @@ private static unsafe void WriteValueSlow(object ptr, int ofs, T val, Action< [MethodImpl(MethodImplOptions.InternalCall)] public static extern void SetLastPInvokeError(int error); - /// - /// Begins tracking system call errors on the calling thread during PAL APIs. The system call errors may include - /// information about the system call made, relevant arguments, return values, and error codes. A call to this method - /// should be followed by a call to on the same thread, which returns the set - /// of system call errors that occurred on the thread in that period. Only system call errors that lead to PAL API - /// failures may be tracked. - /// - [MethodImpl(MethodImplOptions.InternalCall)] - internal static extern void BeginTrackingSystemCallErrors(); - - /// - /// Retrieves system call errors that occurred on the calling thread since - /// was called. - /// - /// Indicates whether to return the accumulated system call errors. - /// - /// System call errors that occurred on the calling thread since was called. - /// Returns null if has not been called, or if no system call - /// errors occurred on the calling thread since it was last called. - /// - [MethodImpl(MethodImplOptions.InternalCall)] - internal static extern string EndTrackingSystemCallErrors(bool getSystemCallErrors); - private static void PrelinkCore(MethodInfo m) { if (!(m is RuntimeMethodInfo rmi)) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Unix.cs b/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Unix.cs new file mode 100644 index 00000000000000..fa025baab9a143 --- /dev/null +++ b/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Unix.cs @@ -0,0 +1,81 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using System.Text; +using Microsoft.Win32.SafeHandles; + +namespace System.Threading +{ + /// + /// Synchronization primitive that can also be used for interprocess synchronization + /// + public sealed partial class Mutex : WaitHandle + { + private const int SystemCallErrorsBufferSize = 256; + + private static unsafe SafeWaitHandle CreateMutexCore( + nint mutexAttributes, + bool initialOwner, + string? name, + uint desiredAccess, + out int errorCode, + out string? errorDetails) + { + byte* systemCallErrors = stackalloc byte[SystemCallErrorsBufferSize]; + SafeWaitHandle mutexHandle = + CreateMutex( + mutexAttributes, + initialOwner, + name, + systemCallErrors, + SystemCallErrorsBufferSize); + + // Get the error code even if the handle is valid, as it could be ERROR_ALREADY_EXISTS, indicating that the mutex + // already exists and was opened + errorCode = Marshal.GetLastPInvokeError(); + + errorDetails = mutexHandle.IsInvalid ? GetErrorDetails(systemCallErrors) : null; + return mutexHandle; + } + + private static unsafe SafeWaitHandle OpenMutexCore( + uint desiredAccess, + bool inheritHandle, + string name, + out int errorCode, + out string? errorDetails) + { + byte* systemCallErrors = stackalloc byte[SystemCallErrorsBufferSize]; + SafeWaitHandle mutexHandle = + OpenMutex(desiredAccess, inheritHandle, name, systemCallErrors, SystemCallErrorsBufferSize); + errorCode = mutexHandle.IsInvalid ? Marshal.GetLastPInvokeError() : Interop.Errors.ERROR_SUCCESS; + errorDetails = mutexHandle.IsInvalid ? GetErrorDetails(systemCallErrors) : null; + return mutexHandle; + } + + private static unsafe string? GetErrorDetails(byte* systemCallErrors) + { + int systemCallErrorsLength = + new ReadOnlySpan(systemCallErrors, SystemCallErrorsBufferSize).IndexOf((byte)'\0'); + if (systemCallErrorsLength > 0) + { + try + { + return + SR.Format(SR.Unix_SystemCallErrors, Encoding.UTF8.GetString(systemCallErrors, systemCallErrorsLength)); + } + catch { } // avoid hiding the original error due to an error here + } + + return null; + } + + [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "PAL_CreateMutexW", SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] + private static unsafe partial SafeWaitHandle CreateMutex(nint mutexAttributes, [MarshalAs(UnmanagedType.Bool)] bool initialOwner, string? name, byte *systemCallErrors, uint systemCallErrorsBufferSize); + + [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "PAL_OpenMutexW", SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] + private static unsafe partial SafeWaitHandle OpenMutex(uint desiredAccess, [MarshalAs(UnmanagedType.Bool)] bool inheritHandle, string name, byte* systemCallErrors, uint systemCallErrorsBufferSize); + } +} diff --git a/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Windows.cs b/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Windows.cs new file mode 100644 index 00000000000000..4170a1d8066f7f --- /dev/null +++ b/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Windows.cs @@ -0,0 +1,50 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.InteropServices; +using Microsoft.Win32.SafeHandles; + +namespace System.Threading +{ + /// + /// Synchronization primitive that can also be used for interprocess synchronization + /// + public sealed partial class Mutex : WaitHandle + { + private static SafeWaitHandle CreateMutexCore( + nint mutexAttributes, + bool initialOwner, + string? name, + uint desiredAccess, + out int errorCode, + out string? errorDetails) + { + errorDetails = null; + SafeWaitHandle mutexHandle = + Interop.Kernel32.CreateMutexEx( + mutexAttributes, + name, + initialOwner ? Interop.Kernel32.CREATE_MUTEX_INITIAL_OWNER : 0, + desiredAccess); + + // Get the error code even if the handle is valid, as it could be ERROR_ALREADY_EXISTS, indicating that the mutex + // already exists and was opened + errorCode = Marshal.GetLastPInvokeError(); + + return mutexHandle; + } + + private static SafeWaitHandle OpenMutexCore( + uint desiredAccess, + bool inheritHandle, + string name, + out int errorCode, + out string? errorDetails) + { + errorDetails = null; + SafeWaitHandle mutexHandle = Interop.Kernel32.OpenMutex(desiredAccess, inheritHandle, name); + errorCode = mutexHandle.IsInvalid ? Marshal.GetLastPInvokeError() : Interop.Errors.ERROR_SUCCESS; + return mutexHandle; + } + } +} diff --git a/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.cs new file mode 100644 index 00000000000000..9f887724d37a06 --- /dev/null +++ b/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.cs @@ -0,0 +1,89 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; +using System.IO; +using System.Runtime.InteropServices; +using Microsoft.Win32.SafeHandles; + +namespace System.Threading +{ + /// + /// Synchronization primitive that can also be used for interprocess synchronization + /// + public sealed partial class Mutex : WaitHandle + { + private const uint AccessRights = + (uint)Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.MUTEX_MODIFY_STATE; + + private void CreateMutexCore(bool initiallyOwned, string? name, out bool createdNew) + { + SafeWaitHandle mutexHandle = + CreateMutexCore(0, initiallyOwned, name, AccessRights, out int errorCode, out string? errorDetails); + + if (mutexHandle.IsInvalid) + { + mutexHandle.SetHandleAsInvalid(); +#if TARGET_UNIX || TARGET_BROWSER || TARGET_WASI + if (errorCode == Interop.Errors.ERROR_FILENAME_EXCED_RANGE) + // On Unix, length validation is done by CoreCLR's PAL after converting to utf-8 + throw new ArgumentException(SR.Argument_WaitHandleNameTooLong, nameof(name)); +#endif + if (errorCode == Interop.Errors.ERROR_INVALID_HANDLE) + throw new WaitHandleCannotBeOpenedException(SR.Format(SR.Threading_WaitHandleCannotBeOpenedException_InvalidHandle, name)); + + throw Win32Marshal.GetExceptionForWin32Error(errorCode, name, errorDetails); + } + + createdNew = errorCode != Interop.Errors.ERROR_ALREADY_EXISTS; + SafeWaitHandle = mutexHandle; + } + + private static OpenExistingResult OpenExistingWorker(string name, out Mutex? result) + { + ArgumentException.ThrowIfNullOrEmpty(name); + + result = null; + // To allow users to view & edit the ACL's, call OpenMutex + // with parameters to allow us to view & edit the ACL. This will + // fail if we don't have permission to view or edit the ACL's. + // If that happens, ask for less permissions. + SafeWaitHandle myHandle = OpenMutexCore(AccessRights, false, name, out int errorCode, out string? errorDetails); + + if (myHandle.IsInvalid) + { + myHandle.Dispose(); + +#if TARGET_UNIX || TARGET_BROWSER || TARGET_WASI + if (errorCode == Interop.Errors.ERROR_FILENAME_EXCED_RANGE) + { + // On Unix, length validation is done by CoreCLR's PAL after converting to utf-8 + throw new ArgumentException(SR.Argument_WaitHandleNameTooLong, nameof(name)); + } +#endif + if (Interop.Errors.ERROR_FILE_NOT_FOUND == errorCode || Interop.Errors.ERROR_INVALID_NAME == errorCode) + return OpenExistingResult.NameNotFound; + if (Interop.Errors.ERROR_PATH_NOT_FOUND == errorCode) + return OpenExistingResult.PathNotFound; + if (Interop.Errors.ERROR_INVALID_HANDLE == errorCode) + return OpenExistingResult.NameInvalid; + + throw Win32Marshal.GetExceptionForWin32Error(errorCode, name, errorDetails); + } + + result = new Mutex(myHandle); + return OpenExistingResult.Success; + } + + // Note: To call ReleaseMutex, you must have an ACL granting you + // MUTEX_MODIFY_STATE rights (0x0001). The other interesting value + // in a Mutex's ACL is MUTEX_ALL_ACCESS (0x1F0001). + public void ReleaseMutex() + { + if (!Interop.Kernel32.ReleaseMutex(SafeWaitHandle)) + { + throw new ApplicationException(SR.Arg_SynchronizationLockException); + } + } + } +} diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.NativeAot.cs index 9a56fd07978b05..a3c5c5d9e1884b 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.NativeAot.cs @@ -210,27 +210,6 @@ public static void SetLastPInvokeError(int error) PInvokeMarshal.t_lastError = error; } - /// - /// Begins tracking system call errors on the calling thread during PAL APIs. The system call errors may include - /// information about the system call made, relevant arguments, return values, and error codes. A call to this method - /// should be followed by a call to on the same thread, which returns the set - /// of system call errors that occurred on the thread in that period. Only system call errors that lead to PAL API - /// failures may be tracked. - /// - internal static void BeginTrackingSystemCallErrors() { } - - /// - /// Retrieves system call errors that occurred on the calling thread since - /// was called. - /// - /// Indicates whether to return the accumulated system call errors. - /// - /// System call errors that occurred on the calling thread since was called. - /// Returns null if has not been called, or if no system call - /// errors occurred on the calling thread since it was last called. - /// - internal static string EndTrackingSystemCallErrors(bool getSystemCallErrors) => null; - internal static bool IsPinnable(object o) { return (o == null) || !o.GetEETypePtr().ContainsGCPointers; diff --git a/src/coreclr/pal/inc/pal.h b/src/coreclr/pal/inc/pal.h index 19f046395a74ab..d856791237c6e2 100644 --- a/src/coreclr/pal/inc/pal.h +++ b/src/coreclr/pal/inc/pal.h @@ -1048,6 +1048,16 @@ CreateMutexExW( IN DWORD dwFlags, IN DWORD dwDesiredAccess); +PALIMPORT +HANDLE +PALAPI +PAL_CreateMutexW( + IN LPSECURITY_ATTRIBUTES lpMutexAttributes, + IN BOOL bInitialOwner, + IN LPCWSTR lpName, + IN LPSTR lpSystemCallErrors, + IN DWORD dwSystemCallErrorsBufferSize); + // CreateMutexExW: dwFlags #define CREATE_MUTEX_INITIAL_OWNER ((DWORD)0x1) @@ -1061,6 +1071,16 @@ OpenMutexW( IN BOOL bInheritHandle, IN LPCWSTR lpName); +PALIMPORT +HANDLE +PALAPI +PAL_OpenMutexW( + IN DWORD dwDesiredAccess, + IN BOOL bInheritHandle, + IN LPCWSTR lpName, + IN LPSTR lpSystemCallErrors, + IN DWORD dwSystemCallErrorsBufferSize); + #ifdef UNICODE #define OpenMutex OpenMutexW #endif @@ -3792,16 +3812,6 @@ PALAPI SetLastError( IN DWORD dwErrCode); -PALIMPORT -VOID -PALAPI -PAL_BeginTrackingSystemCallErrors(); - -PALIMPORT -LPCSTR -PALAPI -PAL_EndTrackingSystemCallErrors(bool getSystemCallErrors); - PALIMPORT LPWSTR PALAPI diff --git a/src/coreclr/pal/src/configure.cmake b/src/coreclr/pal/src/configure.cmake index 304bc3461e9f3f..5366932629c628 100644 --- a/src/coreclr/pal/src/configure.cmake +++ b/src/coreclr/pal/src/configure.cmake @@ -133,6 +133,7 @@ check_function_exists(semget HAS_SYSV_SEMAPHORES) check_function_exists(pthread_mutex_init HAS_PTHREAD_MUTEXES) check_function_exists(ttrace HAVE_TTRACE) check_function_exists(pipe2 HAVE_PIPE2) +check_function_exists(strerrorname_np HAVE_STRERRORNAME_NP) check_cxx_source_compiles(" #include diff --git a/src/coreclr/pal/src/include/pal/mutex.hpp b/src/coreclr/pal/src/include/pal/mutex.hpp index 464f0f72afb454..016668dafb1624 100644 --- a/src/coreclr/pal/src/include/pal/mutex.hpp +++ b/src/coreclr/pal/src/include/pal/mutex.hpp @@ -32,6 +32,7 @@ namespace CorUnix PAL_ERROR InternalCreateMutex( + SharedMemorySystemCallErrors *errors, CPalThread *pThread, LPSECURITY_ATTRIBUTES lpMutexAttributes, BOOL bInitialOwner, @@ -47,6 +48,7 @@ namespace CorUnix PAL_ERROR InternalOpenMutex( + SharedMemorySystemCallErrors *errors, CPalThread *pThread, LPCSTR lpName, HANDLE *phMutex @@ -151,10 +153,10 @@ enum class MutexTryAcquireLockResult class MutexHelpers { public: - static void InitializeProcessSharedRobustRecursiveMutex(pthread_mutex_t *mutex); + static void InitializeProcessSharedRobustRecursiveMutex(SharedMemorySystemCallErrors *errors, pthread_mutex_t *mutex); static void DestroyMutex(pthread_mutex_t *mutex); - static MutexTryAcquireLockResult TryAcquireLock(pthread_mutex_t *mutex, DWORD timeoutMilliseconds); + static MutexTryAcquireLockResult TryAcquireLock(SharedMemorySystemCallErrors *errors, pthread_mutex_t *mutex, DWORD timeoutMilliseconds); static void ReleaseLock(pthread_mutex_t *mutex); }; #endif // NAMED_MUTEX_USE_PTHREAD_MUTEX @@ -172,7 +174,7 @@ class NamedMutexSharedData bool m_isAbandoned; public: - NamedMutexSharedData(); + NamedMutexSharedData(SharedMemorySystemCallErrors *errors); ~NamedMutexSharedData(); #if NAMED_MUTEX_USE_PTHREAD_MUTEX @@ -214,10 +216,10 @@ class NamedMutexProcessData : public SharedMemoryProcessDataBase bool m_hasRefFromLockOwnerThread; public: - static SharedMemoryProcessDataHeader *CreateOrOpen(LPCSTR name, bool acquireLockIfCreated, bool *createdRef); - static SharedMemoryProcessDataHeader *Open(LPCSTR name); + static SharedMemoryProcessDataHeader *CreateOrOpen(SharedMemorySystemCallErrors *errors, LPCSTR name, bool acquireLockIfCreated, bool *createdRef); + static SharedMemoryProcessDataHeader *Open(SharedMemorySystemCallErrors *errors, LPCSTR name); private: - static SharedMemoryProcessDataHeader *CreateOrOpen(LPCSTR name, bool createIfNotExist, bool acquireLockIfCreated, bool *createdRef); + static SharedMemoryProcessDataHeader *CreateOrOpen(SharedMemorySystemCallErrors *errors, LPCSTR name, bool createIfNotExist, bool acquireLockIfCreated, bool *createdRef); public: NamedMutexProcessData( @@ -248,7 +250,7 @@ class NamedMutexProcessData : public SharedMemoryProcessDataBase void SetNextInThreadOwnedNamedMutexList(NamedMutexProcessData *next); public: - MutexTryAcquireLockResult TryAcquireLock(DWORD timeoutMilliseconds); + MutexTryAcquireLockResult TryAcquireLock(SharedMemorySystemCallErrors *errors, DWORD timeoutMilliseconds); void ReleaseLock(); void Abandon(); private: diff --git a/src/coreclr/pal/src/include/pal/sharedmemory.h b/src/coreclr/pal/src/include/pal/sharedmemory.h index be01499586a08b..fce6d2859cc05e 100644 --- a/src/coreclr/pal/src/include/pal/sharedmemory.h +++ b/src/coreclr/pal/src/include/pal/sharedmemory.h @@ -85,6 +85,19 @@ class SharedMemoryException DWORD GetErrorCode() const; }; +class SharedMemorySystemCallErrors +{ +private: + char *m_buffer; + int m_bufferSize; + int m_length; + bool m_isTracking; + +public: + SharedMemorySystemCallErrors(char *buffer, int bufferSize); + void Append(LPCSTR format, ...); +}; + class SharedMemoryHelpers { private: @@ -106,22 +119,22 @@ class SharedMemoryHelpers static void BuildSharedFilesPath(PathCharString& destination, const char *suffix, int suffixByteCount); static bool AppendUInt32String(PathCharString& destination, UINT32 value); - static bool EnsureDirectoryExists(const char *path, bool isGlobalLockAcquired, bool createIfNotExist = true, bool isSystemDirectory = false); + static bool EnsureDirectoryExists(SharedMemorySystemCallErrors *errors, const char *path, bool isGlobalLockAcquired, bool createIfNotExist = true, bool isSystemDirectory = false); private: - static int Open(LPCSTR path, int flags, mode_t mode = static_cast(0)); + static int Open(SharedMemorySystemCallErrors *errors, LPCSTR path, int flags, mode_t mode = static_cast(0)); public: - static int OpenDirectory(LPCSTR path); - static int CreateOrOpenFile(LPCSTR path, bool createIfNotExist = true, bool *createdRef = nullptr); + static int OpenDirectory(SharedMemorySystemCallErrors *errors, LPCSTR path); + static int CreateOrOpenFile(SharedMemorySystemCallErrors *errors, LPCSTR path, bool createIfNotExist = true, bool *createdRef = nullptr); static void CloseFile(int fileDescriptor); static int ChangeMode(LPCSTR path, mode_t mode); - static SIZE_T GetFileSize(LPCSTR filePath, int fileDescriptor); - static void SetFileSize(LPCSTR filePath, int fileDescriptor, SIZE_T byteCount); + static SIZE_T GetFileSize(SharedMemorySystemCallErrors *errors, LPCSTR filePath, int fileDescriptor); + static void SetFileSize(SharedMemorySystemCallErrors *errors, LPCSTR filePath, int fileDescriptor, SIZE_T byteCount); - static void *MemoryMapFile(LPCSTR filePath, int fileDescriptor, SIZE_T byteCount); + static void *MemoryMapFile(SharedMemorySystemCallErrors *errors, LPCSTR filePath, int fileDescriptor, SIZE_T byteCount); - static bool TryAcquireFileLock(int fileDescriptor, int operation); + static bool TryAcquireFileLock(SharedMemorySystemCallErrors *errors, int fileDescriptor, int operation); static void ReleaseFileLock(int fileDescriptor); static void VerifyStringOperation(bool success); @@ -209,7 +222,7 @@ class SharedMemoryProcessDataHeader SharedMemoryProcessDataHeader *m_nextInProcessDataHeaderList; public: - static SharedMemoryProcessDataHeader *CreateOrOpen(LPCSTR name, SharedMemorySharedDataHeader requiredSharedDataHeader, SIZE_T sharedDataByteCount, bool createIfNotExist, bool *createdRef); + static SharedMemoryProcessDataHeader *CreateOrOpen(SharedMemorySystemCallErrors *errors, LPCSTR name, SharedMemorySharedDataHeader requiredSharedDataHeader, SIZE_T sharedDataByteCount, bool createIfNotExist, bool *createdRef); public: static SharedMemoryProcessDataHeader *PalObject_GetProcessDataHeader(CorUnix::IPalObject *object); @@ -262,7 +275,7 @@ class SharedMemoryManager public: static void AcquireCreationDeletionProcessLock(); static void ReleaseCreationDeletionProcessLock(); - static void AcquireCreationDeletionFileLock(); + static void AcquireCreationDeletionFileLock(SharedMemorySystemCallErrors *errors); static void ReleaseCreationDeletionFileLock(); public: diff --git a/src/coreclr/pal/src/include/pal/thread.hpp b/src/coreclr/pal/src/include/pal/thread.hpp index 4932f084edbce4..f9a357eacddc55 100644 --- a/src/coreclr/pal/src/include/pal/thread.hpp +++ b/src/coreclr/pal/src/include/pal/thread.hpp @@ -467,22 +467,6 @@ namespace CorUnix return errno; }; - void - BeginTrackingSystemCallErrors( - void - ); - - static void - AppendSystemCallError( - LPCSTR format, - ... - ); - - LPCSTR - EndTrackingSystemCallErrors( - bool getSystemCallErrors - ); - void SetExitCode( DWORD dwExitCode diff --git a/src/coreclr/pal/src/include/pal/utils.h b/src/coreclr/pal/src/include/pal/utils.h index 2a7ffc3d2ef854..fdd5b3b965a167 100644 --- a/src/coreclr/pal/src/include/pal/utils.h +++ b/src/coreclr/pal/src/include/pal/utils.h @@ -213,5 +213,4 @@ class StringHolder }; #endif /* _PAL_UTILS_H_ */ -const int RawErrorCodeStringBufferSize = 12; -const char *GetFriendlyErrorCodeString(int errorCode, char (&rawErrorCodeStringBuffer)[RawErrorCodeStringBufferSize]); +const char *GetFriendlyErrorCodeString(int errorCode); diff --git a/src/coreclr/pal/src/misc/error.cpp b/src/coreclr/pal/src/misc/error.cpp index f4a30aa4700aca..6b67bf717d52cb 100644 --- a/src/coreclr/pal/src/misc/error.cpp +++ b/src/coreclr/pal/src/misc/error.cpp @@ -84,70 +84,3 @@ SetLastError( CPalThread::SetLastError(dwErrCode); } -/*++ -Function: - PAL_BeginTrackingSystemCallErrors - -PAL_BeginTrackingSystemCallErrors - -The PAL_BeginTrackingSystemCallErrors function begins tracking system call errors on the calling thread during PAL APIs. The -system call errors may include information about the system call made, relevant arguments, return values, and error codes. A -call to this function should be followed by a call to PAL_EndTrackingSystemCallErrors on the same thread, which returns the set -of system call errors that occurred on the thread in that period. This may not track all system call errors, it may only track -system call errors that directly or indirectly lead to PAL API failures. - -Parameters - -This function has no parameters. - -Return Values - -This function does not return a value. - ---*/ -VOID -PALAPI -PAL_BeginTrackingSystemCallErrors( - VOID) -{ - CPalThread *thread = GetCurrentPalThread(); - if (thread != nullptr) - { - thread->BeginTrackingSystemCallErrors(); - } -} - -/*++ -Function: - PAL_EndTrackingSystemCallErrors - -PAL_EndTrackingSystemCallErrors - -The PAL_EndTrackingSystemCallErrors function retrieves system call errors that occurred on the calling thread since -PAL_BeginTrackingSystemCallErrors was called. - -Parameters - -This function has no parameters. - -Return Values - -The return value is the system call errors that occurred on the calling thread since PAL_BeginTrackingSystemCallErrors was -called. Returns NULL if PAL_BeginTrackingSystemCallErrors has not been called, or if no system call errors occurred on the -calling thread since it was last called. If the returned pointer is not NULL, it is only safe to be used by the calling thread -and before the next call to PAL_BeginTrackingSystemCallErrors. - ---*/ -LPCSTR -PALAPI -PAL_EndTrackingSystemCallErrors( - bool getSystemCallErrors) -{ - CPalThread *thread = GetCurrentPalThread(); - if (thread != nullptr) - { - return thread->EndTrackingSystemCallErrors(getSystemCallErrors); - } - - return nullptr; -} diff --git a/src/coreclr/pal/src/misc/utils.cpp b/src/coreclr/pal/src/misc/utils.cpp index 2c2bb4f57ab123..0d96cc991305ae 100644 --- a/src/coreclr/pal/src/misc/utils.cpp +++ b/src/coreclr/pal/src/misc/utils.cpp @@ -367,11 +367,24 @@ BOOL IsRunningOnMojaveHardenedRuntime() #endif // __APPLE__ -const char *GetFriendlyErrorCodeString(int errorCode, char (&rawErrorCodeStringBuffer)[RawErrorCodeStringBufferSize]) +const char *GetFriendlyErrorCodeString(int errorCode) { +#if HAVE_STRERRORNAME_NP + const char *error = strerrorname_np(errorCode); + if (error != nullptr) + { + return error; + } +#else // !HAVE_STRERRORNAME_NP switch (errorCode) { case EACCES: return "EACCES"; + #if EAGAIN == EWOULDBLOCK + case EAGAIN: return "EAGAIN/EWOULDBLOCK"; + #else + case EAGAIN: return "EAGAIN"; + case EWOULDBLOCK: return "EWOULDBLOCK"; + #endif case EBADF: return "EBADF"; case EBUSY: return "EBUSY"; case EDQUOT: return "EDQUOT"; @@ -392,6 +405,12 @@ const char *GetFriendlyErrorCodeString(int errorCode, char (&rawErrorCodeStringB case ENOLCK: return "ENOLCK"; case ENOMEM: return "ENOMEM"; case ENOSPC: return "ENOSPC"; + #if ENOTSUP == EOPNOTSUPP + case ENOTSUP: return "ENOTSUP/EOPNOTSUPP"; + #else + case ENOTSUP: return "ENOTSUP"; + case EOPNOTSUPP: return "EOPNOTSUPP"; + #endif case ENOTDIR: return "ENOTDIR"; case ENOTEMPTY: return "ENOTEMPTY"; case ENXIO: return "ENXIO"; @@ -401,26 +420,7 @@ const char *GetFriendlyErrorCodeString(int errorCode, char (&rawErrorCodeStringB case ETXTBSY: return "ETXTBSY"; case EXDEV: return "EXDEV"; } +#endif // HAVE_STRERRORNAME_NP - if (errorCode == EAGAIN || errorCode == EWOULDBLOCK) - { - if (EAGAIN == EWOULDBLOCK) return "EAGAIN/EWOULDBLOCK"; - if (errorCode == EAGAIN) return "EAGAIN"; - return "EWOULDBLOCK"; - } - else if (errorCode == ENOTSUP || errorCode == EOPNOTSUPP) - { - if (ENOTSUP == EOPNOTSUPP) return "ENOTSUP/EOPNOTSUPP"; - if (errorCode == ENOTSUP) return "ENOTSUP"; - return "EOPNOTSUPP"; - } - - int result = - _snprintf_s(rawErrorCodeStringBuffer, RawErrorCodeStringBufferSize, RawErrorCodeStringBufferSize - 1, "%d", errorCode); - if (result <= 0 || result >= RawErrorCodeStringBufferSize) - { - rawErrorCodeStringBuffer[0] = '\0'; - } - - return rawErrorCodeStringBuffer; + return strerror(errorCode); } diff --git a/src/coreclr/pal/src/sharedmemory/sharedmemory.cpp b/src/coreclr/pal/src/sharedmemory/sharedmemory.cpp index cc78831ac2240d..ea5aae444dad00 100644 --- a/src/coreclr/pal/src/sharedmemory/sharedmemory.cpp +++ b/src/coreclr/pal/src/sharedmemory/sharedmemory.cpp @@ -60,6 +60,71 @@ DWORD SharedMemoryException::GetErrorCode() const return m_errorCode; } +//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// SharedMemorySystemCallErrors + +SharedMemorySystemCallErrors::SharedMemorySystemCallErrors(char *buffer, int bufferSize) + : m_buffer(buffer), m_bufferSize(bufferSize), m_length(0), m_isTracking(bufferSize != 0) +{ + _ASSERTE((buffer == nullptr) == (bufferSize == 0)); + _ASSERTE(bufferSize >= 0); +} + +void SharedMemorySystemCallErrors::Append(LPCSTR format, ...) +{ + if (!m_isTracking) + { + return; + } + + char *buffer = m_buffer; + _ASSERTE(buffer != nullptr); + int bufferSize = m_bufferSize; + _ASSERTE(bufferSize != 0); + int length = m_length; + _ASSERTE(length < bufferSize); + _ASSERTE(buffer[length] == '\0'); + if (length >= bufferSize - 1) + { + return; + } + + if (length != 0) + { + length++; // the previous null terminator will be changed to a space if the append succeeds + } + + va_list args; + va_start(args, format); + int result = _vsnprintf_s(buffer + length, bufferSize - length, bufferSize - 1 - length, format, args); + va_end(args); + + if (result == 0) + { + return; + } + + if (result < 0 || result >= bufferSize - length) + { + // There's not enough space to append this error, discard the append and stop tracking + if (length == 0) + { + buffer[0] = '\0'; + } + m_isTracking = false; + return; + } + + if (length != 0) + { + buffer[length - 1] = ' '; // change the previous null terminator to a space + } + + length += result; + _ASSERTE(buffer[length] == '\0'); + m_length = length; +} + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // SharedMemoryHelpers @@ -95,6 +160,7 @@ SIZE_T SharedMemoryHelpers::AlignUp(SIZE_T value, SIZE_T alignment) } bool SharedMemoryHelpers::EnsureDirectoryExists( + SharedMemorySystemCallErrors *errors, const char *path, bool isGlobalLockAcquired, bool createIfNotExist, @@ -105,8 +171,6 @@ bool SharedMemoryHelpers::EnsureDirectoryExists( _ASSERTE(SharedMemoryManager::IsCreationDeletionProcessLockAcquired()); _ASSERTE(!isGlobalLockAcquired || SharedMemoryManager::IsCreationDeletionFileLockAcquired()); - char rawErrorCodeStringBuffer[RawErrorCodeStringBufferSize]; - // Check if the path already exists struct stat statInfo; int statResult = stat(path, &statInfo); @@ -129,26 +193,32 @@ bool SharedMemoryHelpers::EnsureDirectoryExists( int operationResult = mkdir(path, PermissionsMask_AllUsers_ReadWriteExecute); if (operationResult != 0) { - int errorCode = errno; - CPalThread::AppendSystemCallError( - "mkdir(\"%s\", 0x%x) == %d; errno == %s;", - path, - (int)PermissionsMask_AllUsers_ReadWriteExecute, - operationResult, - GetFriendlyErrorCodeString(errorCode, rawErrorCodeStringBuffer)); + if (errors != nullptr) + { + int errorCode = errno; + errors->Append( + "mkdir(\"%s\", AllUsers_ReadWriteExecute) == %d; errno == %s;", + path, + operationResult, + GetFriendlyErrorCodeString(errorCode)); + } + throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } operationResult = ChangeMode(path, PermissionsMask_AllUsers_ReadWriteExecute); if (operationResult != 0) { - int errorCode = errno; - CPalThread::AppendSystemCallError( - "chmod(\"%s\", 0x%x) == %d; errno == %s;", - path, - (int)PermissionsMask_AllUsers_ReadWriteExecute, - operationResult, - GetFriendlyErrorCodeString(errorCode, rawErrorCodeStringBuffer)); + if (errors != nullptr) + { + int errorCode = errno; + errors->Append( + "chmod(\"%s\", AllUsers_ReadWriteExecute) == %d; errno == %s;", + path, + operationResult, + GetFriendlyErrorCodeString(errorCode)); + } + rmdir(path); throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } @@ -161,24 +231,31 @@ bool SharedMemoryHelpers::EnsureDirectoryExists( if (mkdtemp(tempPath.OpenStringBuffer()) == nullptr) { - int errorCode = errno; - CPalThread::AppendSystemCallError( - "mkdtemp(\"%s\") == nullptr; errno == %s;", - (const char *)tempPath, - GetFriendlyErrorCodeString(errorCode, rawErrorCodeStringBuffer)); + if (errors != nullptr) + { + int errorCode = errno; + errors->Append( + "mkdtemp(\"%s\") == nullptr; errno == %s;", + (const char *)tempPath, + GetFriendlyErrorCodeString(errorCode)); + } + throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } int operationResult = ChangeMode(tempPath, PermissionsMask_AllUsers_ReadWriteExecute); if (operationResult != 0) { - int errorCode = errno; - CPalThread::AppendSystemCallError( - "chmod(\"%s\", 0x%x) == %d; errno == %s;", - (const char *)tempPath, - (int)PermissionsMask_AllUsers_ReadWriteExecute, - operationResult, - GetFriendlyErrorCodeString(errorCode, rawErrorCodeStringBuffer)); + if (errors != nullptr) + { + int errorCode = errno; + errors->Append( + "chmod(\"%s\", AllUsers_ReadWriteExecute) == %d; errno == %s;", + (const char *)tempPath, + operationResult, + GetFriendlyErrorCodeString(errorCode)); + } + rmdir(tempPath); throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } @@ -197,22 +274,25 @@ bool SharedMemoryHelpers::EnsureDirectoryExists( // If the path exists, check that it's a directory if (statResult != 0 || !(statInfo.st_mode & S_IFDIR)) { - if (statResult != 0) + if (errors != nullptr) { - int errorCode = errno; - CPalThread::AppendSystemCallError( - "stat(\"%s\", ...) == %d; errno == %s;", - path, - statResult, - GetFriendlyErrorCodeString(errorCode, rawErrorCodeStringBuffer)); - } - else - { - CPalThread::AppendSystemCallError( - "stat(\"%s\", &info) == 0; info.st_mode == 0x%x; (info.st_mode & 0x%x) == 0;", - path, - (int)statInfo.st_mode, - (int)S_IFDIR); + if (statResult != 0) + { + int errorCode = errno; + errors->Append( + "stat(\"%s\", ...) == %d; errno == %s;", + path, + statResult, + GetFriendlyErrorCodeString(errorCode)); + } + else + { + errors->Append( + "stat(\"%s\", &info) == 0; info.st_mode == 0x%x; (info.st_mode & 0x%x) == 0;", + path, + (int)statInfo.st_mode, + (int)S_IFDIR); + } } throw SharedMemoryException(static_cast(SharedMemoryError::IO)); @@ -231,12 +311,14 @@ bool SharedMemoryHelpers::EnsureDirectoryExists( return true; } - CPalThread::AppendSystemCallError( - "stat(\"%s\", &info) == 0; info.st_mode == 0x%x; (info.st_mode & 0x%x) != 0x%x;", - path, - (int)statInfo.st_mode, - (int)PermissionsMask_CurrentUser_ReadWriteExecute, - (int)PermissionsMask_CurrentUser_ReadWriteExecute); + if (errors != nullptr) + { + errors->Append( + "stat(\"%s\", &info) == 0; info.st_mode == 0x%x; (info.st_mode & CurrentUser_ReadWriteExecute) != CurrentUser_ReadWriteExecute;", + path, + (int)statInfo.st_mode); + } + throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } @@ -253,19 +335,21 @@ bool SharedMemoryHelpers::EnsureDirectoryExists( // As a last resort, check that at least the current user has full access. if ((statInfo.st_mode & PermissionsMask_CurrentUser_ReadWriteExecute) != PermissionsMask_CurrentUser_ReadWriteExecute) { - CPalThread::AppendSystemCallError( - "stat(\"%s\", &info) == 0; info.st_mode == 0x%x; (info.st_mode & 0x%x) != 0x%x;", - path, - (int)statInfo.st_mode, - (int)PermissionsMask_CurrentUser_ReadWriteExecute, - (int)PermissionsMask_CurrentUser_ReadWriteExecute); + if (errors != nullptr) + { + errors->Append( + "stat(\"%s\", &info) == 0; info.st_mode == 0x%x; (info.st_mode & CurrentUser_ReadWriteExecute) != CurrentUser_ReadWriteExecute;", + path, + (int)statInfo.st_mode); + } + throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } } return true; } -int SharedMemoryHelpers::Open(LPCSTR path, int flags, mode_t mode) +int SharedMemoryHelpers::Open(SharedMemorySystemCallErrors *errors, LPCSTR path, int flags, mode_t mode) { int openErrorCode; @@ -303,31 +387,34 @@ int SharedMemoryHelpers::Open(LPCSTR path, int flags, mode_t mode) break; } - if (sharedMemoryError != SharedMemoryError::NameTooLong) + if (sharedMemoryError != SharedMemoryError::NameTooLong && errors != nullptr) { - char rawErrorCodeStringBuffer[RawErrorCodeStringBufferSize]; - CPalThread::AppendSystemCallError( + errors->Append( "open(\"%s\", 0x%x, 0x%x) == -1; errno == %s;", path, flags, (int)mode, - GetFriendlyErrorCodeString(openErrorCode, rawErrorCodeStringBuffer)); + GetFriendlyErrorCodeString(openErrorCode)); } throw SharedMemoryException(static_cast(sharedMemoryError)); } -int SharedMemoryHelpers::OpenDirectory(LPCSTR path) +int SharedMemoryHelpers::OpenDirectory(SharedMemorySystemCallErrors *errors, LPCSTR path) { _ASSERTE(path != nullptr); _ASSERTE(path[0] != '\0'); - int fileDescriptor = Open(path, O_RDONLY); + int fileDescriptor = Open(errors, path, O_RDONLY); _ASSERTE(fileDescriptor != -1 || errno == ENOENT); return fileDescriptor; } -int SharedMemoryHelpers::CreateOrOpenFile(LPCSTR path, bool createIfNotExist, bool *createdRef) +int SharedMemoryHelpers::CreateOrOpenFile( + SharedMemorySystemCallErrors *errors, + LPCSTR path, + bool createIfNotExist, + bool *createdRef) { _ASSERTE(path != nullptr); _ASSERTE(path[0] != '\0'); @@ -336,7 +423,7 @@ int SharedMemoryHelpers::CreateOrOpenFile(LPCSTR path, bool createIfNotExist, bo // Try to open the file int openFlags = O_RDWR; - int fileDescriptor = Open(path, openFlags); + int fileDescriptor = Open(errors, path, openFlags); if (fileDescriptor != -1) { if (createdRef != nullptr) @@ -357,7 +444,7 @@ int SharedMemoryHelpers::CreateOrOpenFile(LPCSTR path, bool createIfNotExist, bo // File does not exist, create the file openFlags |= O_CREAT | O_EXCL; - fileDescriptor = Open(path, openFlags, PermissionsMask_AllUsers_ReadWrite); + fileDescriptor = Open(errors, path, openFlags, PermissionsMask_AllUsers_ReadWrite); _ASSERTE(fileDescriptor != -1); // The permissions mask passed to open() is filtered by the process' permissions umask, so open() may not set all of @@ -365,14 +452,16 @@ int SharedMemoryHelpers::CreateOrOpenFile(LPCSTR path, bool createIfNotExist, bo int operationResult = ChangeMode(path, PermissionsMask_AllUsers_ReadWrite); if (operationResult != 0) { - int errorCode = errno; - char rawErrorCodeStringBuffer[RawErrorCodeStringBufferSize]; - CPalThread::AppendSystemCallError( - "chmod(\"%s\", 0x%x) == %d; errno == %s;", - path, - (int)PermissionsMask_AllUsers_ReadWrite, - operationResult, - GetFriendlyErrorCodeString(errorCode, rawErrorCodeStringBuffer)); + if (errors != nullptr) + { + int errorCode = errno; + errors->Append( + "chmod(\"%s\", AllUsers_ReadWrite) == %d; errno == %s;", + path, + operationResult, + GetFriendlyErrorCodeString(errorCode)); + } + CloseFile(fileDescriptor); unlink(path); throw SharedMemoryException(static_cast(SharedMemoryError::IO)); @@ -410,7 +499,7 @@ int SharedMemoryHelpers::ChangeMode(LPCSTR path, mode_t mode) return chmodResult; } -SIZE_T SharedMemoryHelpers::GetFileSize(LPCSTR filePath, int fileDescriptor) +SIZE_T SharedMemoryHelpers::GetFileSize(SharedMemorySystemCallErrors *errors, LPCSTR filePath, int fileDescriptor) { _ASSERTE(filePath != nullptr); _ASSERTE(filePath[0] != '\0'); @@ -420,20 +509,27 @@ SIZE_T SharedMemoryHelpers::GetFileSize(LPCSTR filePath, int fileDescriptor) if (endOffset == static_cast(-1) || lseek(fileDescriptor, 0, SEEK_SET) == static_cast(-1)) { - int errorCode = errno; - char rawErrorCodeStringBuffer[RawErrorCodeStringBufferSize]; - CPalThread::AppendSystemCallError( - "lseek(\"%s\", 0, %s) == -1; errno == %s;", - filePath, - (int)PermissionsMask_AllUsers_ReadWrite, - GetFriendlyErrorCodeString(errorCode, rawErrorCodeStringBuffer)); + if (errors != nullptr) + { + int errorCode = errno; + errors->Append( + "lseek(\"%s\", 0, %s) == -1; errno == %s;", + filePath, + endOffset == (off_t)-1 ? "SEEK_END" : "SEEK_SET", + GetFriendlyErrorCodeString(errorCode)); + } + throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } return endOffset; } -void SharedMemoryHelpers::SetFileSize(LPCSTR filePath, int fileDescriptor, SIZE_T byteCount) +void SharedMemoryHelpers::SetFileSize( + SharedMemorySystemCallErrors *errors, + LPCSTR filePath, + int fileDescriptor, + SIZE_T byteCount) { _ASSERTE(filePath != nullptr); _ASSERTE(filePath[0] != '\0'); @@ -451,19 +547,26 @@ void SharedMemoryHelpers::SetFileSize(LPCSTR filePath, int fileDescriptor, SIZE_ int errorCode = errno; if (errorCode != EINTR) { - char rawErrorCodeStringBuffer[RawErrorCodeStringBufferSize]; - CPalThread::AppendSystemCallError( - "ftruncate(\"%s\", %zu) == %d; errno == %s;", - filePath, - byteCount, - ftruncateResult, - GetFriendlyErrorCodeString(errorCode, rawErrorCodeStringBuffer)); + if (errors != nullptr) + { + errors->Append( + "ftruncate(\"%s\", %zu) == %d; errno == %s;", + filePath, + byteCount, + ftruncateResult, + GetFriendlyErrorCodeString(errorCode)); + } + throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } } } -void *SharedMemoryHelpers::MemoryMapFile(LPCSTR filePath, int fileDescriptor, SIZE_T byteCount) +void *SharedMemoryHelpers::MemoryMapFile( + SharedMemorySystemCallErrors *errors, + LPCSTR filePath, + int fileDescriptor, + SIZE_T byteCount) { _ASSERTE(filePath != nullptr); _ASSERTE(filePath[0] != '\0'); @@ -492,16 +595,19 @@ void *SharedMemoryHelpers::MemoryMapFile(LPCSTR filePath, int fileDescriptor, SI break; } - char rawErrorCodeStringBuffer[RawErrorCodeStringBufferSize]; - CPalThread::AppendSystemCallError( - "mmap(nullptr, %zu, PROT_READ | PROT_WRITE, MAP_SHARED, \"%s\", 0) == MAP_FAILED; errno == %s;", - byteCount, - filePath, - GetFriendlyErrorCodeString(errorCode, rawErrorCodeStringBuffer)); + if (errors != nullptr) + { + errors->Append( + "mmap(nullptr, %zu, PROT_READ | PROT_WRITE, MAP_SHARED, \"%s\", 0) == MAP_FAILED; errno == %s;", + byteCount, + filePath, + GetFriendlyErrorCodeString(errorCode)); + } + throw SharedMemoryException(static_cast(sharedMemoryError)); } -bool SharedMemoryHelpers::TryAcquireFileLock(int fileDescriptor, int operation) +bool SharedMemoryHelpers::TryAcquireFileLock(SharedMemorySystemCallErrors *errors, int fileDescriptor, int operation) { // A file lock is acquired once per file descriptor, so the caller will need to synchronize threads of this process @@ -532,14 +638,17 @@ bool SharedMemoryHelpers::TryAcquireFileLock(int fileDescriptor, int operation) break; } - char rawErrorCodeStringBuffer[RawErrorCodeStringBufferSize]; - CPalThread::AppendSystemCallError( - "flock(%d, %s%s) == %d; errno == %s;", - fileDescriptor, - operation & LOCK_EX ? "LOCK_EX" : "LOCK_SH", - operation & LOCK_NB ? " | LOCK_NB" : "", - flockResult, - GetFriendlyErrorCodeString(flockError, rawErrorCodeStringBuffer)); + if (errors != nullptr) + { + errors->Append( + "flock(%d, %s%s) == %d; errno == %s;", + fileDescriptor, + operation & LOCK_EX ? "LOCK_EX" : "LOCK_SH", + operation & LOCK_NB ? " | LOCK_NB" : "", + flockResult, + GetFriendlyErrorCodeString(flockError)); + } + throw SharedMemoryException(static_cast(sharedMemoryError)); } } @@ -716,6 +825,7 @@ void *SharedMemorySharedDataHeader::GetData() // SharedMemoryProcessDataHeader SharedMemoryProcessDataHeader *SharedMemoryProcessDataHeader::CreateOrOpen( + SharedMemorySystemCallErrors *errors, LPCSTR name, SharedMemorySharedDataHeader requiredSharedDataHeader, SIZE_T sharedDataByteCount, @@ -815,14 +925,14 @@ SharedMemoryProcessDataHeader *SharedMemoryProcessDataHeader::CreateOrOpen( return processDataHeader; } - SharedMemoryManager::AcquireCreationDeletionFileLock(); + SharedMemoryManager::AcquireCreationDeletionFileLock(errors); autoCleanup.m_acquiredCreationDeletionFileLock = true; // Create the session directory SharedMemoryHelpers::VerifyStringOperation(SharedMemoryManager::CopySharedMemoryBasePath(filePath)); SharedMemoryHelpers::VerifyStringOperation(filePath.Append('/')); SharedMemoryHelpers::VerifyStringOperation(id.AppendSessionDirectoryName(filePath)); - if (!SharedMemoryHelpers::EnsureDirectoryExists(filePath, true /* isGlobalLockAcquired */, createIfNotExist)) + if (!SharedMemoryHelpers::EnsureDirectoryExists(errors, filePath, true /* isGlobalLockAcquired */, createIfNotExist)) { _ASSERTE(!createIfNotExist); return nullptr; @@ -835,7 +945,7 @@ SharedMemoryProcessDataHeader *SharedMemoryProcessDataHeader::CreateOrOpen( SharedMemoryHelpers::VerifyStringOperation(filePath.Append(id.GetName(), id.GetNameCharCount())); bool createdFile; - int fileDescriptor = SharedMemoryHelpers::CreateOrOpenFile(filePath, createIfNotExist, &createdFile); + int fileDescriptor = SharedMemoryHelpers::CreateOrOpenFile(errors, filePath, createIfNotExist, &createdFile); if (fileDescriptor == -1) { _ASSERTE(!createIfNotExist); @@ -850,7 +960,7 @@ SharedMemoryProcessDataHeader *SharedMemoryProcessDataHeader::CreateOrOpen( // A shared file lock on the shared memory file would be held by any process that has opened the same file. Try to take // an exclusive lock on the file. Successfully acquiring an exclusive lock indicates that no process has a reference to // the shared memory file, and this process can reinitialize its contents. - if (SharedMemoryHelpers::TryAcquireFileLock(fileDescriptor, LOCK_EX | LOCK_NB)) + if (SharedMemoryHelpers::TryAcquireFileLock(errors, fileDescriptor, LOCK_EX | LOCK_NB)) { // The shared memory file is not being used, flag it as created so that its contents will be reinitialized SharedMemoryHelpers::ReleaseFileLock(fileDescriptor); @@ -869,18 +979,18 @@ SharedMemoryProcessDataHeader *SharedMemoryProcessDataHeader::CreateOrOpen( SIZE_T sharedDataTotalByteCount = SharedMemorySharedDataHeader::GetTotalByteCount(sharedDataByteCount); if (createdFile) { - SharedMemoryHelpers::SetFileSize(filePath, fileDescriptor, sharedDataTotalByteCount); + SharedMemoryHelpers::SetFileSize(errors, filePath, fileDescriptor, sharedDataTotalByteCount); } else { - SIZE_T currentFileSize = SharedMemoryHelpers::GetFileSize(filePath, fileDescriptor); + SIZE_T currentFileSize = SharedMemoryHelpers::GetFileSize(errors, filePath, fileDescriptor); if (currentFileSize < sharedDataUsedByteCount) { throw SharedMemoryException(static_cast(SharedMemoryError::HeaderMismatch)); } if (currentFileSize < sharedDataTotalByteCount) { - SharedMemoryHelpers::SetFileSize(filePath, fileDescriptor, sharedDataTotalByteCount); + SharedMemoryHelpers::SetFileSize(errors, filePath, fileDescriptor, sharedDataTotalByteCount); } } @@ -888,20 +998,23 @@ SharedMemoryProcessDataHeader *SharedMemoryProcessDataHeader::CreateOrOpen( // using the file. An exclusive file lock is attempted above to detect whether the file contents are valid, for the case // where a process crashes or is killed after the file is created. Since we already hold the creation/deletion locks, a // non-blocking file lock should succeed. - if (!SharedMemoryHelpers::TryAcquireFileLock(fileDescriptor, LOCK_SH | LOCK_NB)) + if (!SharedMemoryHelpers::TryAcquireFileLock(errors, fileDescriptor, LOCK_SH | LOCK_NB)) { - int errorCode = errno; - char rawErrorCodeStringBuffer[RawErrorCodeStringBufferSize]; - CPalThread::AppendSystemCallError( - "flock(\"%s\", LOCK_SH | LOCK_NB) == -1; errno == %s;", - (const char *)filePath, - GetFriendlyErrorCodeString(errorCode, rawErrorCodeStringBuffer)); + if (errors != nullptr) + { + int errorCode = errno; + errors->Append( + "flock(\"%s\", LOCK_SH | LOCK_NB) == -1; errno == %s;", + (const char *)filePath, + GetFriendlyErrorCodeString(errorCode)); + } + throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } autoCleanup.m_acquiredFileLock = true; // Map the file into memory, and initialize or validate the header - void *mappedBuffer = SharedMemoryHelpers::MemoryMapFile(filePath, fileDescriptor, sharedDataTotalByteCount); + void *mappedBuffer = SharedMemoryHelpers::MemoryMapFile(errors, filePath, fileDescriptor, sharedDataTotalByteCount); autoCleanup.m_mappedBuffer = mappedBuffer; autoCleanup.m_mappedBufferByteCount = sharedDataTotalByteCount; SharedMemorySharedDataHeader *sharedDataHeader; @@ -1090,11 +1203,11 @@ void SharedMemoryProcessDataHeader::Close() bool releaseSharedData = false; try { - SharedMemoryManager::AcquireCreationDeletionFileLock(); + SharedMemoryManager::AcquireCreationDeletionFileLock(nullptr); autoReleaseCreationDeletionFileLock.m_acquired = true; SharedMemoryHelpers::ReleaseFileLock(m_fileDescriptor); - if (SharedMemoryHelpers::TryAcquireFileLock(m_fileDescriptor, LOCK_EX | LOCK_NB)) + if (SharedMemoryHelpers::TryAcquireFileLock(nullptr, m_fileDescriptor, LOCK_EX | LOCK_NB)) { SharedMemoryHelpers::ReleaseFileLock(m_fileDescriptor); releaseSharedData = true; @@ -1306,7 +1419,7 @@ void SharedMemoryManager::ReleaseCreationDeletionProcessLock() LeaveCriticalSection(&s_creationDeletionProcessLock); } -void SharedMemoryManager::AcquireCreationDeletionFileLock() +void SharedMemoryManager::AcquireCreationDeletionFileLock(SharedMemorySystemCallErrors *errors) { _ASSERTE(IsCreationDeletionProcessLockAcquired()); _ASSERTE(!IsCreationDeletionFileLockAcquired()); @@ -1314,38 +1427,48 @@ void SharedMemoryManager::AcquireCreationDeletionFileLock() if (s_creationDeletionLockFileDescriptor == -1) { if (!SharedMemoryHelpers::EnsureDirectoryExists( + errors, *gSharedFilesPath, false /* isGlobalLockAcquired */, false /* createIfNotExist */, true /* isSystemDirectory */)) { _ASSERTE(errno == ENOENT); - CPalThread::AppendSystemCallError("stat(\"%s\", ...) == -1; errno == ENOENT;", (const char *)*gSharedFilesPath); + if (errors != nullptr) + { + errors->Append("stat(\"%s\", ...) == -1; errno == ENOENT;", (const char *)*gSharedFilesPath); + } + throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } SharedMemoryHelpers::EnsureDirectoryExists( + errors, *s_runtimeTempDirectoryPath, false /* isGlobalLockAcquired */); SharedMemoryHelpers::EnsureDirectoryExists( + errors, *s_sharedMemoryDirectoryPath, false /* isGlobalLockAcquired */); - s_creationDeletionLockFileDescriptor = SharedMemoryHelpers::OpenDirectory(*s_sharedMemoryDirectoryPath); + s_creationDeletionLockFileDescriptor = SharedMemoryHelpers::OpenDirectory(errors, *s_sharedMemoryDirectoryPath); if (s_creationDeletionLockFileDescriptor == -1) { - int errorCode = errno; - char rawErrorCodeStringBuffer[RawErrorCodeStringBufferSize]; - CPalThread::AppendSystemCallError( - "open(\"%s\", O_RDONLY | O_CLOEXEC, 0) == -1; errno == %s;", - (const char *)*s_sharedMemoryDirectoryPath, - GetFriendlyErrorCodeString(errorCode, rawErrorCodeStringBuffer)); + if (errors != nullptr) + { + int errorCode = errno; + errors->Append( + "open(\"%s\", O_RDONLY | O_CLOEXEC, 0) == -1; errno == %s;", + (const char *)*s_sharedMemoryDirectoryPath, + GetFriendlyErrorCodeString(errorCode)); + } + throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } } - bool acquiredFileLock = SharedMemoryHelpers::TryAcquireFileLock(s_creationDeletionLockFileDescriptor, LOCK_EX); + bool acquiredFileLock = SharedMemoryHelpers::TryAcquireFileLock(errors, s_creationDeletionLockFileDescriptor, LOCK_EX); _ASSERTE(acquiredFileLock); #ifdef _DEBUG s_creationDeletionFileLockOwnerThreadId = THREADSilentGetCurrentThreadId(); diff --git a/src/coreclr/pal/src/synchmgr/wait.cpp b/src/coreclr/pal/src/synchmgr/wait.cpp index e4bab9a6fb74c0..b44f6abbb7ca18 100644 --- a/src/coreclr/pal/src/synchmgr/wait.cpp +++ b/src/coreclr/pal/src/synchmgr/wait.cpp @@ -440,7 +440,7 @@ DWORD CorUnix::InternalWaitForMultipleObjectsEx( try { MutexTryAcquireLockResult tryAcquireLockResult = - static_cast(processDataHeader->GetData())->TryAcquireLock(dwMilliseconds); + static_cast(processDataHeader->GetData())->TryAcquireLock(nullptr, dwMilliseconds); switch (tryAcquireLockResult) { case MutexTryAcquireLockResult::AcquiredLock: diff --git a/src/coreclr/pal/src/synchobj/mutex.cpp b/src/coreclr/pal/src/synchobj/mutex.cpp index 0da0fa8f73b50c..aacbc648f5d318 100644 --- a/src/coreclr/pal/src/synchobj/mutex.cpp +++ b/src/coreclr/pal/src/synchobj/mutex.cpp @@ -93,33 +93,74 @@ static CAllowedObjectTypes aotAnyMutex(anyMutexTypeIds, ARRAY_SIZE(anyMutexTypeI Function: CreateMutexW + See doc for PAL_CreateMutexW. +--*/ + +HANDLE +PALAPI +CreateMutexW( + IN LPSECURITY_ATTRIBUTES lpMutexAttributes, + IN BOOL bInitialOwner, + IN LPCWSTR lpName) +{ + return PAL_CreateMutexW(lpMutexAttributes, bInitialOwner, lpName, nullptr, 0); +} + +/*++ +Function: + PAL_CreateMutexW + Note: lpMutexAttributes currently ignored: -- Win32 object security not supported -- handles to mutex objects are not inheritable Parameters: - See MSDN doc. + lpSystemCallErrors -- An optional buffer into which system call errors are written, for more detailed error information. + dwSystemCallErrorsBufferSize -- Size of the buffer pointed to by lpSystemCallErrors in bytes. + + See MSDN docs on CreateMutexW for all other parameters. --*/ HANDLE PALAPI -CreateMutexW( +PAL_CreateMutexW( IN LPSECURITY_ATTRIBUTES lpMutexAttributes, IN BOOL bInitialOwner, - IN LPCWSTR lpName) + IN LPCWSTR lpName, + IN LPSTR lpSystemCallErrors, + IN DWORD dwSystemCallErrorsBufferSize) { HANDLE hMutex = NULL; PAL_ERROR palError; CPalThread *pthr = NULL; char utf8Name[SHARED_MEMORY_MAX_NAME_CHAR_COUNT + 1]; - PERF_ENTRY(CreateMutexW); - ENTRY("CreateMutexW(lpMutexAttr=%p, bInitialOwner=%d, lpName=%p (%S)\n", - lpMutexAttributes, bInitialOwner, lpName, lpName?lpName:W16_NULLSTRING); + PERF_ENTRY(PAL_CreateMutexW); + ENTRY("PAL_CreateMutexW(lpMutexAttr=%p, bInitialOwner=%d, lpName=%p (%S), " + "lpSystemCallErrors=%p, dwSystemCallErrorsBufferSize=%d\n", + lpMutexAttributes, + bInitialOwner, + lpName, + lpName?lpName:W16_NULLSTRING, + lpSystemCallErrors, + dwSystemCallErrorsBufferSize); pthr = InternalGetCurrentThread(); + /* validate parameters */ + if ((int)dwSystemCallErrorsBufferSize < 0 || (lpSystemCallErrors == nullptr) != (dwSystemCallErrorsBufferSize == 0)) + { + ERROR("One or more parameters are invalid\n"); + palError = ERROR_INVALID_PARAMETER; + goto CreateMutexWExit; + } + + if (lpSystemCallErrors != nullptr) + { + lpSystemCallErrors[0] = '\0'; + } + if (lpName != nullptr) { int bytesWritten = WideCharToMultiByte(CP_ACP, 0, lpName, -1, utf8Name, ARRAY_SIZE(utf8Name), nullptr, nullptr); @@ -139,13 +180,17 @@ CreateMutexW( } } - palError = InternalCreateMutex( - pthr, - lpMutexAttributes, - bInitialOwner, - lpName == nullptr ? nullptr : utf8Name, - &hMutex - ); + { + SharedMemorySystemCallErrors errors(lpSystemCallErrors, (int)dwSystemCallErrorsBufferSize); + palError = InternalCreateMutex( + &errors, + pthr, + lpMutexAttributes, + bInitialOwner, + lpName == nullptr ? nullptr : utf8Name, + &hMutex + ); + } CreateMutexWExit: // @@ -157,14 +202,14 @@ CreateMutexW( pthr->SetLastError(palError); - LOGEXIT("CreateMutexW returns HANDLE %p\n", hMutex); - PERF_EXIT(CreateMutexW); + LOGEXIT("PAL_CreateMutexW returns HANDLE %p\n", hMutex); + PERF_EXIT(PAL_CreateMutexW); return hMutex; } /*++ Function: -CreateMutexW +CreateMutexExW Note: lpMutexAttributes currently ignored: @@ -196,14 +241,16 @@ CreateMutexExW( -- handles to mutex objects are not inheritable Parameters: + errors -- An optional wrapper for system call errors, for more detailed error information. pthr -- thread data for calling thread phEvent -- on success, receives the allocated mutex handle - See MSDN docs on CreateMutex for all other parameters + See MSDN docs on CreateMutex for all other parameters. --*/ PAL_ERROR CorUnix::InternalCreateMutex( + SharedMemorySystemCallErrors *errors, CPalThread *pthr, LPSECURITY_ATTRIBUTES lpMutexAttributes, BOOL bInitialOwner, @@ -287,7 +334,7 @@ CorUnix::InternalCreateMutex( SharedMemoryProcessDataHeader *processDataHeader; try { - processDataHeader = NamedMutexProcessData::CreateOrOpen(lpName, !!bInitialOwner, &createdNamedMutex); + processDataHeader = NamedMutexProcessData::CreateOrOpen(errors, lpName, !!bInitialOwner, &createdNamedMutex); } catch (SharedMemoryException ex) { @@ -513,7 +560,7 @@ OpenMutexA ( goto OpenMutexAExit; } - palError = InternalOpenMutex(pthr, lpName, &hMutex); + palError = InternalOpenMutex(nullptr, pthr, lpName, &hMutex); OpenMutexAExit: if (NO_ERROR != palError) @@ -530,39 +577,76 @@ OpenMutexA ( Function: OpenMutexW +Parameters: + See doc for PAL_OpenMutexW. +--*/ + +HANDLE +PALAPI +OpenMutexW( + IN DWORD dwDesiredAccess, + IN BOOL bInheritHandle, + IN LPCWSTR lpName) +{ + return PAL_OpenMutexW(dwDesiredAccess, bInheritHandle, lpName, nullptr, 0); +} + +/*++ +Function: + PAL_OpenMutexW + Note: dwDesiredAccess is currently ignored (no Win32 object security support) bInheritHandle is currently ignored (handles to mutexes are not inheritable) -See MSDN doc. +Parameters: + lpSystemCallErrors -- An optional buffer into which system call errors are written, for more detailed error information. + dwSystemCallErrorsBufferSize -- Size of the buffer pointed to by lpSystemCallErrors in bytes. + + See MSDN docs on OpenMutexW for all other parameters. --*/ HANDLE PALAPI -OpenMutexW( +PAL_OpenMutexW( IN DWORD dwDesiredAccess, IN BOOL bInheritHandle, - IN LPCWSTR lpName) + IN LPCWSTR lpName, + IN LPSTR lpSystemCallErrors, + IN DWORD dwSystemCallErrorsBufferSize) { HANDLE hMutex = NULL; PAL_ERROR palError = NO_ERROR; CPalThread *pthr = NULL; char utf8Name[SHARED_MEMORY_MAX_NAME_CHAR_COUNT + 1]; - PERF_ENTRY(OpenMutexW); - ENTRY("OpenMutexW(dwDesiredAccess=%#x, bInheritHandle=%d, lpName=%p (%S))\n", - dwDesiredAccess, bInheritHandle, lpName, lpName?lpName:W16_NULLSTRING); + PERF_ENTRY(PAL_OpenMutexW); + ENTRY("PAL_OpenMutexW(dwDesiredAccess=%#x, bInheritHandle=%d, lpName=%p (%S), " + "lpSystemCallErrors=%p, dwSystemCallErrorsBufferSize=%d)\n", + dwDesiredAccess, + bInheritHandle, + lpName, + lpName?lpName:W16_NULLSTRING, + lpSystemCallErrors, + dwSystemCallErrorsBufferSize); pthr = InternalGetCurrentThread(); /* validate parameters */ - if (lpName == nullptr) + if (lpName == nullptr || + (int)dwSystemCallErrorsBufferSize < 0 || + (lpSystemCallErrors == nullptr) != (dwSystemCallErrorsBufferSize == 0)) { - ERROR("name is NULL\n"); + ERROR("name is NULL or other parameters are invalid\n"); palError = ERROR_INVALID_PARAMETER; goto OpenMutexWExit; } + if (lpSystemCallErrors != nullptr) + { + lpSystemCallErrors[0] = '\0'; + } + { int bytesWritten = WideCharToMultiByte(CP_ACP, 0, lpName, -1, utf8Name, ARRAY_SIZE(utf8Name), nullptr, nullptr); if (bytesWritten == 0) @@ -579,9 +663,10 @@ OpenMutexW( } goto OpenMutexWExit; } - } - palError = InternalOpenMutex(pthr, lpName == nullptr ? nullptr : utf8Name, &hMutex); + SharedMemorySystemCallErrors errors(lpSystemCallErrors, (int)dwSystemCallErrorsBufferSize); + palError = InternalOpenMutex(&errors, pthr, lpName == nullptr ? nullptr : utf8Name, &hMutex); + } OpenMutexWExit: if (NO_ERROR != palError) @@ -589,8 +674,8 @@ OpenMutexW( pthr->SetLastError(palError); } - LOGEXIT("OpenMutexW returns HANDLE %p\n", hMutex); - PERF_EXIT(OpenMutexW); + LOGEXIT("PAL_OpenMutexW returns HANDLE %p\n", hMutex); + PERF_EXIT(PAL_OpenMutexW); return hMutex; } @@ -600,6 +685,7 @@ OpenMutexW( InternalOpenMutex Parameters: + errors -- An optional wrapper for system call errors, for more detailed error information. pthr -- thread data for calling thread phEvent -- on success, receives the allocated mutex handle @@ -608,6 +694,7 @@ OpenMutexW( PAL_ERROR CorUnix::InternalOpenMutex( + SharedMemorySystemCallErrors *errors, CPalThread *pthr, LPCSTR lpName, HANDLE *phMutex @@ -646,7 +733,7 @@ CorUnix::InternalOpenMutex( SharedMemoryProcessDataHeader *processDataHeader; try { - processDataHeader = NamedMutexProcessData::Open(lpName); + processDataHeader = NamedMutexProcessData::Open(errors, lpName); } catch (SharedMemoryException ex) { @@ -747,12 +834,10 @@ DWORD SPINLOCKTryAcquire (LONG * lock) // MutexHelpers #if NAMED_MUTEX_USE_PTHREAD_MUTEX -void MutexHelpers::InitializeProcessSharedRobustRecursiveMutex(pthread_mutex_t *mutex) +void MutexHelpers::InitializeProcessSharedRobustRecursiveMutex(SharedMemorySystemCallErrors *errors, pthread_mutex_t *mutex) { _ASSERTE(mutex != nullptr); - char rawErrorCodeStringBuffer[RawErrorCodeStringBufferSize]; - struct AutoCleanup { pthread_mutexattr_t *m_mutexAttributes; @@ -775,9 +860,11 @@ void MutexHelpers::InitializeProcessSharedRobustRecursiveMutex(pthread_mutex_t * int error = pthread_mutexattr_init(&mutexAttributes); if (error != 0) { - CPalThread::AppendSystemCallError( - "pthread_mutexattr_init(...) == %d;", - GetFriendlyErrorCodeString(error, rawErrorCodeStringBuffer)); + if (errors != nullptr) + { + errors->Append("pthread_mutexattr_init(...) == %s;", GetFriendlyErrorCodeString(error)); + } + throw SharedMemoryException(static_cast(SharedMemoryError::OutOfMemory)); } autoCleanup.m_mutexAttributes = &mutexAttributes; @@ -794,9 +881,11 @@ void MutexHelpers::InitializeProcessSharedRobustRecursiveMutex(pthread_mutex_t * error = pthread_mutex_init(mutex, &mutexAttributes); if (error != 0) { - CPalThread::AppendSystemCallError( - "pthread_mutex_init(...) == %d;", - GetFriendlyErrorCodeString(error, rawErrorCodeStringBuffer)); + if (errors != nullptr) + { + errors->Append("pthread_mutex_init(...) == %s;", GetFriendlyErrorCodeString(error)); + } + throw SharedMemoryException(static_cast(error == EPERM ? SharedMemoryError::IO : SharedMemoryError::OutOfMemory)); } } @@ -809,7 +898,10 @@ void MutexHelpers::DestroyMutex(pthread_mutex_t *mutex) _ASSERTE(error == 0 || error == EBUSY); // the error will be EBUSY if the mutex is locked } -MutexTryAcquireLockResult MutexHelpers::TryAcquireLock(pthread_mutex_t *mutex, DWORD timeoutMilliseconds) +MutexTryAcquireLockResult MutexHelpers::TryAcquireLock( + SharedMemorySystemCallErrors *errors, + pthread_mutex_t *mutex, + DWORD timeoutMilliseconds) { _ASSERTE(mutex != nullptr); @@ -860,13 +952,16 @@ MutexTryAcquireLockResult MutexHelpers::TryAcquireLock(pthread_mutex_t *mutex, D default: { - char rawErrorCodeStringBuffer[RawErrorCodeStringBufferSize]; - CPalThread::AppendSystemCallError( - "%s(...) == %d;", - timeoutMilliseconds == (DWORD)-1 ? "pthread_mutex_lock" - : timeoutMilliseconds == 0 ? "pthread_mutex_trylock" - : "pthread_mutex_timedlock", - GetFriendlyErrorCodeString(lockResult, rawErrorCodeStringBuffer)); + if (errors != nullptr) + { + errors->Append( + "%s(...) == %s;", + timeoutMilliseconds == (DWORD)-1 ? "pthread_mutex_lock" + : timeoutMilliseconds == 0 ? "pthread_mutex_trylock" + : "pthread_mutex_timedlock", + GetFriendlyErrorCodeString(lockResult)); + } + throw SharedMemoryException(static_cast(NamedMutexError::Unknown)); } } @@ -884,7 +979,7 @@ void MutexHelpers::ReleaseLock(pthread_mutex_t *mutex) //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // NamedMutexSharedData -NamedMutexSharedData::NamedMutexSharedData() +NamedMutexSharedData::NamedMutexSharedData(SharedMemorySystemCallErrors *errors) : #if !NAMED_MUTEX_USE_PTHREAD_MUTEX m_timedWaiterCount(0), @@ -901,7 +996,7 @@ NamedMutexSharedData::NamedMutexSharedData() _ASSERTE(SharedMemoryManager::IsCreationDeletionFileLockAcquired()); #if NAMED_MUTEX_USE_PTHREAD_MUTEX - MutexHelpers::InitializeProcessSharedRobustRecursiveMutex(&m_lock); + MutexHelpers::InitializeProcessSharedRobustRecursiveMutex(errors, &m_lock); #endif // NAMED_MUTEX_USE_PTHREAD_MUTEX } @@ -998,17 +1093,22 @@ const UINT8 NamedMutexProcessData::SyncSystemVersion = 1; const DWORD NamedMutexProcessData::PollLoopMaximumSleepMilliseconds = 100; -SharedMemoryProcessDataHeader *NamedMutexProcessData::CreateOrOpen(LPCSTR name, bool acquireLockIfCreated, bool *createdRef) +SharedMemoryProcessDataHeader *NamedMutexProcessData::CreateOrOpen( + SharedMemorySystemCallErrors *errors, + LPCSTR name, + bool acquireLockIfCreated, + bool *createdRef) { - return CreateOrOpen(name, true /* createIfNotExist */, acquireLockIfCreated, createdRef); + return CreateOrOpen(errors, name, true /* createIfNotExist */, acquireLockIfCreated, createdRef); } -SharedMemoryProcessDataHeader *NamedMutexProcessData::Open(LPCSTR name) +SharedMemoryProcessDataHeader *NamedMutexProcessData::Open(SharedMemorySystemCallErrors *errors, LPCSTR name) { - return CreateOrOpen(name, false /* createIfNotExist */, false /* acquireLockIfCreated */, nullptr /* createdRef */); + return CreateOrOpen(errors, name, false /* createIfNotExist */, false /* acquireLockIfCreated */, nullptr /* createdRef */); } SharedMemoryProcessDataHeader *NamedMutexProcessData::CreateOrOpen( + SharedMemorySystemCallErrors *errors, LPCSTR name, bool createIfNotExist, bool acquireLockIfCreated, @@ -1098,6 +1198,7 @@ SharedMemoryProcessDataHeader *NamedMutexProcessData::CreateOrOpen( bool created; SharedMemoryProcessDataHeader *processDataHeader = SharedMemoryProcessDataHeader::CreateOrOpen( + errors, name, SharedMemorySharedDataHeader(SharedMemoryType::Mutex, SyncSystemVersion), sizeof(NamedMutexSharedData), @@ -1124,7 +1225,7 @@ SharedMemoryProcessDataHeader *NamedMutexProcessData::CreateOrOpen( if (created) { // Initialize the shared data - new(processDataHeader->GetSharedDataHeader()->GetData()) NamedMutexSharedData; + new(processDataHeader->GetSharedDataHeader()->GetData()) NamedMutexSharedData(errors); } if (processDataHeader->GetData() == nullptr) @@ -1134,7 +1235,7 @@ SharedMemoryProcessDataHeader *NamedMutexProcessData::CreateOrOpen( SharedMemoryHelpers::BuildSharedFilesPath(lockFilePath, SHARED_MEMORY_LOCK_FILES_DIRECTORY_NAME); if (created) { - SharedMemoryHelpers::EnsureDirectoryExists(lockFilePath, true /* isGlobalLockAcquired */); + SharedMemoryHelpers::EnsureDirectoryExists(errors, lockFilePath, true /* isGlobalLockAcquired */); } // Create the session directory @@ -1143,7 +1244,7 @@ SharedMemoryProcessDataHeader *NamedMutexProcessData::CreateOrOpen( SharedMemoryHelpers::VerifyStringOperation(id->AppendSessionDirectoryName(lockFilePath)); if (created) { - SharedMemoryHelpers::EnsureDirectoryExists(lockFilePath, true /* isGlobalLockAcquired */); + SharedMemoryHelpers::EnsureDirectoryExists(errors, lockFilePath, true /* isGlobalLockAcquired */); autoCleanup.m_lockFilePath = &lockFilePath; autoCleanup.m_sessionDirectoryPathCharCount = lockFilePath.GetCount(); } @@ -1151,15 +1252,19 @@ SharedMemoryProcessDataHeader *NamedMutexProcessData::CreateOrOpen( // Create or open the lock file SharedMemoryHelpers::VerifyStringOperation(lockFilePath.Append('/')); SharedMemoryHelpers::VerifyStringOperation(lockFilePath.Append(id->GetName(), id->GetNameCharCount())); - int lockFileDescriptor = SharedMemoryHelpers::CreateOrOpenFile(lockFilePath, created); + int lockFileDescriptor = SharedMemoryHelpers::CreateOrOpenFile(errors, lockFilePath, created); if (lockFileDescriptor == -1) { _ASSERTE(!created); if (createIfNotExist) { - CPalThread::AppendSystemCallError( - "open(\"%s\", O_RDWR | O_CREAT | O_EXCL | O_CLOEXEC, 0) == -1; errno == ENOENT;", - (const char *)lockFilePath); + if (errors != nullptr) + { + errors->Append( + "open(\"%s\", O_RDWR | O_CREAT | O_EXCL | O_CLOEXEC, 0) == -1; errno == ENOENT;", + (const char *)lockFilePath); + } + throw SharedMemoryException(static_cast(SharedMemoryError::IO)); } @@ -1187,7 +1292,7 @@ SharedMemoryProcessDataHeader *NamedMutexProcessData::CreateOrOpen( // If the mutex was created and if requested, acquire the lock initially while holding the creation/deletion locks if (created && acquireLockIfCreated) { - MutexTryAcquireLockResult tryAcquireLockResult = processData->TryAcquireLock(0); + MutexTryAcquireLockResult tryAcquireLockResult = processData->TryAcquireLock(errors, 0); _ASSERTE(tryAcquireLockResult == MutexTryAcquireLockResult::AcquiredLock); } } @@ -1354,12 +1459,12 @@ void NamedMutexProcessData::SetNextInThreadOwnedNamedMutexList(NamedMutexProcess m_nextInThreadOwnedNamedMutexList = next; } -MutexTryAcquireLockResult NamedMutexProcessData::TryAcquireLock(DWORD timeoutMilliseconds) +MutexTryAcquireLockResult NamedMutexProcessData::TryAcquireLock(SharedMemorySystemCallErrors *errors, DWORD timeoutMilliseconds) { NamedMutexSharedData *sharedData = GetSharedData(); #if NAMED_MUTEX_USE_PTHREAD_MUTEX - MutexTryAcquireLockResult result = MutexHelpers::TryAcquireLock(sharedData->GetLock(), timeoutMilliseconds); + MutexTryAcquireLockResult result = MutexHelpers::TryAcquireLock(errors, sharedData->GetLock(), timeoutMilliseconds); if (result == MutexTryAcquireLockResult::TimedOut) { return result; @@ -1468,7 +1573,7 @@ MutexTryAcquireLockResult NamedMutexProcessData::TryAcquireLock(DWORD timeoutMil bool acquiredFileLock = false; while (sharedData->HasAnyTimedWaiters()) { - if (SharedMemoryHelpers::TryAcquireFileLock(m_sharedLockFileDescriptor, LOCK_EX | LOCK_NB)) + if (SharedMemoryHelpers::TryAcquireFileLock(errors, m_sharedLockFileDescriptor, LOCK_EX | LOCK_NB)) { acquiredFileLock = true; break; @@ -1480,13 +1585,13 @@ MutexTryAcquireLockResult NamedMutexProcessData::TryAcquireLock(DWORD timeoutMil break; } - acquiredFileLock = SharedMemoryHelpers::TryAcquireFileLock(m_sharedLockFileDescriptor, LOCK_EX); + acquiredFileLock = SharedMemoryHelpers::TryAcquireFileLock(errors, m_sharedLockFileDescriptor, LOCK_EX); _ASSERTE(acquiredFileLock); break; } case 0: - if (!SharedMemoryHelpers::TryAcquireFileLock(m_sharedLockFileDescriptor, LOCK_EX | LOCK_NB)) + if (!SharedMemoryHelpers::TryAcquireFileLock(errors, m_sharedLockFileDescriptor, LOCK_EX | LOCK_NB)) { return MutexTryAcquireLockResult::TimedOut; } @@ -1495,7 +1600,7 @@ MutexTryAcquireLockResult NamedMutexProcessData::TryAcquireLock(DWORD timeoutMil default: { // Try to acquire the file lock without waiting - if (SharedMemoryHelpers::TryAcquireFileLock(m_sharedLockFileDescriptor, LOCK_EX | LOCK_NB)) + if (SharedMemoryHelpers::TryAcquireFileLock(errors, m_sharedLockFileDescriptor, LOCK_EX | LOCK_NB)) { break; } @@ -1534,7 +1639,7 @@ MutexTryAcquireLockResult NamedMutexProcessData::TryAcquireLock(DWORD timeoutMil ? remainingMilliseconds : PollLoopMaximumSleepMilliseconds; Sleep(sleepMilliseconds); - } while (!SharedMemoryHelpers::TryAcquireFileLock(m_sharedLockFileDescriptor, LOCK_EX | LOCK_NB)); + } while (!SharedMemoryHelpers::TryAcquireFileLock(errors, m_sharedLockFileDescriptor, LOCK_EX | LOCK_NB)); break; } } diff --git a/src/coreclr/pal/src/thread/thread.cpp b/src/coreclr/pal/src/thread/thread.cpp index eeab89b23fd4f4..9c3fea9ce81a4b 100644 --- a/src/coreclr/pal/src/thread/thread.cpp +++ b/src/coreclr/pal/src/thread/thread.cpp @@ -2701,96 +2701,6 @@ CPalThread::GetCachedStackLimit() return m_stackLimit; } -void -CPalThread::BeginTrackingSystemCallErrors() -{ - _ASSERTE(!m_isTrackingSystemCallErrors); - - m_isTrackingSystemCallErrors = true; - m_systemCallErrorsLength = 0; - if (m_systemCallErrors != nullptr) - { - m_systemCallErrors[0] = '\0'; - } -} - -void -CPalThread::AppendSystemCallError( - LPCSTR format, - ... - ) -{ - CPalThread *thread = GetCurrentPalThread(); - if (thread == nullptr || !thread->m_isTrackingSystemCallErrors) - { - return; - } - - const int BufferSize = 256; - char *buffer = thread->m_systemCallErrors; - if (buffer == nullptr) - { - buffer = new(std::nothrow) char[BufferSize]; - if (buffer == nullptr) - { - return; - } - - buffer[0] = '\0'; - thread->m_systemCallErrors = buffer; - } - - int length = thread->m_systemCallErrorsLength; - _ASSERTE(length < BufferSize); - _ASSERTE(buffer[length] == '\0'); - if (length >= BufferSize - 1) - { - return; - } - - if (length != 0) - { - length++; // the previous null terminator will be changed to a space if the append succeeds - } - - va_list args; - va_start(args, format); - int result = _vsnprintf_s(buffer + length, BufferSize - length, BufferSize - 1 - length, format, args); - va_end(args); - - if (result == 0) - { - return; - } - - if (result < 0 || result >= BufferSize - length) - { - // There's not enough space to append this error, discard the append and stop tracking system call errors - if (length == 0) - { - buffer[0] = '\0'; - } - thread->m_isTrackingSystemCallErrors = false; - return; - } - - if (length != 0) - { - buffer[length - 1] = ' '; // change the previous null terminator to a space - } - - length += result; - _ASSERTE(buffer[length] == '\0'); - thread->m_systemCallErrorsLength = length; -} - -LPCSTR -CPalThread::EndTrackingSystemCallErrors(bool getSystemCallErrors) -{ - m_isTrackingSystemCallErrors = false; - return getSystemCallErrors ? m_systemCallErrors : nullptr; -} - PVOID PALAPI PAL_GetStackBase() diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index 1b2f1a2fec24bd..c8c83502cfbab2 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/src/coreclr/vm/ecalllist.h @@ -474,8 +474,6 @@ FCFuncEnd() FCFuncStart(gInteropMarshalFuncs) FCFuncElement("GetLastPInvokeError", MarshalNative::GetLastPInvokeError) FCFuncElement("SetLastPInvokeError", MarshalNative::SetLastPInvokeError) - FCFuncElement("BeginTrackingSystemCallErrors", MarshalNative::BeginTrackingSystemCallErrors) - FCFuncElement("EndTrackingSystemCallErrors", MarshalNative::EndTrackingSystemCallErrors) FCFuncElement("SizeOfHelper", MarshalNative::SizeOfClass) FCFuncElement("StructureToPtr", MarshalNative::StructureToPtr) FCFuncElement("PtrToStructureHelper", MarshalNative::PtrToStructureHelper) diff --git a/src/coreclr/vm/marshalnative.cpp b/src/coreclr/vm/marshalnative.cpp index b936113494dd39..90ea48c1796e5f 100644 --- a/src/coreclr/vm/marshalnative.cpp +++ b/src/coreclr/vm/marshalnative.cpp @@ -446,43 +446,6 @@ FCIMPL1(void, MarshalNative::SetLastPInvokeError, int error) } FCIMPLEND -/************************************************************************ - * Marshal.BeginTrackingSystemCallErrors - */ -FCIMPL0(void, MarshalNative::BeginTrackingSystemCallErrors) -{ - FCALL_CONTRACT; - -#ifdef TARGET_UNIX - PAL_BeginTrackingSystemCallErrors(); -#endif // TARGET_UNIX -} -FCIMPLEND - -/************************************************************************ - * Marshal.EndTrackingSystemCallErrors - */ -FCIMPL1(StringObject *, MarshalNative::EndTrackingSystemCallErrors, CLR_BOOL getSystemCallErrors) -{ - FCALL_CONTRACT; - -#ifdef TARGET_UNIX - STRINGREF errorsRef = NULL; - LPCUTF8 errorsBuffer = PAL_EndTrackingSystemCallErrors(getSystemCallErrors); - if (errorsBuffer != NULL) - { - HELPER_METHOD_FRAME_BEGIN_RET_0(); - errorsRef = StringObject::NewString(errorsBuffer); - HELPER_METHOD_FRAME_END(); - } - - return (StringObject *)OBJECTREFToObject(errorsRef); -#else // !TARGET_UNIX - return NULL; -#endif // TARGET_UNIX -} -FCIMPLEND - /************************************************************************ * Support for the GCHandle class. */ diff --git a/src/coreclr/vm/marshalnative.h b/src/coreclr/vm/marshalnative.h index e5615f0c30ea77..cb26bf2733fb39 100644 --- a/src/coreclr/vm/marshalnative.h +++ b/src/coreclr/vm/marshalnative.h @@ -33,8 +33,6 @@ class MarshalNative static FCDECL1(UINT32, OffsetOfHelper, ReflectFieldObject* pFieldUNSAFE); static FCDECL0(int, GetLastPInvokeError); static FCDECL1(void, SetLastPInvokeError, int error); - static FCDECL0(void, BeginTrackingSystemCallErrors); - static FCDECL1(StringObject *, EndTrackingSystemCallErrors, CLR_BOOL getSystemCallErrors); static FCDECL3(VOID, StructureToPtr, Object* pObjUNSAFE, LPVOID ptr, CLR_BOOL fDeleteOld); static FCDECL3(VOID, PtrToStructureHelper, LPVOID ptr, Object* pObjIn, CLR_BOOL allowValueClasses); diff --git a/src/coreclr/vm/qcallentrypoints.cpp b/src/coreclr/vm/qcallentrypoints.cpp index 0725bd7a87f097..a26a312bd76484 100644 --- a/src/coreclr/vm/qcallentrypoints.cpp +++ b/src/coreclr/vm/qcallentrypoints.cpp @@ -307,6 +307,8 @@ static const Entry s_QCall[] = DllImportEntry(OpenMutexW) DllImportEntry(OpenSemaphoreW) DllImportEntry(OutputDebugStringW) + DllImportEntry(PAL_CreateMutexW) + DllImportEntry(PAL_OpenMutexW) DllImportEntry(ReleaseMutex) DllImportEntry(ReleaseSemaphore) DllImportEntry(ResetEvent) diff --git a/src/libraries/Common/src/System/IO/Win32Marshal.cs b/src/libraries/Common/src/System/IO/Win32Marshal.cs index ad2c81e9fc5158..1c68364b89b86e 100644 --- a/src/libraries/Common/src/System/IO/Win32Marshal.cs +++ b/src/libraries/Common/src/System/IO/Win32Marshal.cs @@ -3,6 +3,7 @@ using System.Diagnostics; using System.Runtime.InteropServices; +using Microsoft.Win32.SafeHandles; namespace System.IO { @@ -22,7 +23,7 @@ internal static Exception GetExceptionForLastWin32Error(string? path = "") /// Converts the specified Win32 error into a corresponding object, optionally /// including the specified path in the error message. /// - internal static Exception GetExceptionForWin32Error(int errorCode, string? path = "") + internal static Exception GetExceptionForWin32Error(int errorCode, string? path = "", string? errorDetails = null) { // ERROR_SUCCESS gets thrown when another unexpected interop call was made before checking GetLastWin32Error(). // Errors have to get retrieved as soon as possible after P/Invoking to avoid this. @@ -57,13 +58,19 @@ internal static Exception GetExceptionForWin32Error(int errorCode, string? path case Interop.Errors.ERROR_OPERATION_ABORTED: return new OperationCanceledException(); case Interop.Errors.ERROR_INVALID_PARAMETER: + default: - string msg = string.IsNullOrEmpty(path) - ? GetPInvokeErrorMessage(errorCode) - : $"{GetPInvokeErrorMessage(errorCode)} : '{path}'"; - return new IOException( - msg, - MakeHRFromErrorCode(errorCode)); + string msg = GetPInvokeErrorMessage(errorCode); + if (!string.IsNullOrEmpty(path)) + { + msg += $" : '{path}'."; + } + if (!string.IsNullOrEmpty(errorDetails)) + { + msg += $" {errorDetails}"; + } + + return new IOException(msg, MakeHRFromErrorCode(errorCode)); } static string GetPInvokeErrorMessage(int errorCode) diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx index 23a0edb40102e2..e1cc39aeaf4aa5 100644 --- a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx +++ b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx @@ -3461,9 +3461,6 @@ in {0}:line {1} - - One or more system calls failed. Details: {0} - Method cannot be invoked. @@ -3638,6 +3635,9 @@ Unknown error "{0}". + + One or more system calls failed: {0} + Operation could destabilize the runtime. diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index 0d6ae242ae6ec6..202d51338929b5 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -2148,6 +2148,7 @@ + @@ -2205,7 +2206,6 @@ Common\System\Memory\FixedBufferExtensions.cs - @@ -2716,4 +2716,4 @@ - \ No newline at end of file + diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Mutex.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Mutex.Windows.cs index 3d86556a66edf8..07eb21b0849805 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Mutex.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Mutex.Windows.cs @@ -4,7 +4,6 @@ using System.IO; using Microsoft.Win32.SafeHandles; using System.Runtime.InteropServices; -using System.Diagnostics; namespace System.Threading { @@ -19,33 +18,16 @@ public sealed partial class Mutex : WaitHandle private void CreateMutexCore(bool initiallyOwned, string? name, out bool createdNew) { uint mutexFlags = initiallyOwned ? Interop.Kernel32.CREATE_MUTEX_INITIAL_OWNER : 0; - SafeWaitHandle mutexHandle = null; - int errorCode; - string? systemCallErrors; - Marshal.BeginTrackingSystemCallErrors(); - try - { - mutexHandle = Interop.Kernel32.CreateMutexEx(IntPtr.Zero, name, mutexFlags, AccessRights); - errorCode = Marshal.GetLastPInvokeError(); - } - finally - { - systemCallErrors = - Marshal.EndTrackingSystemCallErrors(getSystemCallErrors: mutexHandle != null && mutexHandle.IsInvalid); - } + SafeWaitHandle mutexHandle = Interop.Kernel32.CreateMutexEx(IntPtr.Zero, name, mutexFlags, AccessRights); + int errorCode = Marshal.GetLastPInvokeError(); if (mutexHandle.IsInvalid) { mutexHandle.SetHandleAsInvalid(); -#if TARGET_UNIX || TARGET_BROWSER || TARGET_WASI - if (errorCode == Interop.Errors.ERROR_FILENAME_EXCED_RANGE) - // On Unix, length validation is done by CoreCLR's PAL after converting to utf-8 - throw new ArgumentException(SR.Argument_WaitHandleNameTooLong, nameof(name)); -#endif if (errorCode == Interop.Errors.ERROR_INVALID_HANDLE) throw new WaitHandleCannotBeOpenedException(SR.Format(SR.Threading_WaitHandleCannotBeOpenedException_InvalidHandle, name)); - throw GetExceptionForWin32ErrorWithSystemCallError(errorCode, name, systemCallErrors); + throw Win32Marshal.GetExceptionForWin32Error(errorCode, name); } createdNew = errorCode != Interop.Errors.ERROR_ALREADY_EXISTS; @@ -57,39 +39,18 @@ private static OpenExistingResult OpenExistingWorker(string name, out Mutex? res ArgumentException.ThrowIfNullOrEmpty(name); result = null; - SafeWaitHandle myHandle = null; - int errorCode = 0; - string? systemCallErrors; - Marshal.BeginTrackingSystemCallErrors(); - try - { - // To allow users to view & edit the ACL's, call OpenMutex - // with parameters to allow us to view & edit the ACL. This will - // fail if we don't have permission to view or edit the ACL's. - // If that happens, ask for less permissions. - myHandle = Interop.Kernel32.OpenMutex(AccessRights, false, name); - if (myHandle.IsInvalid) - { - errorCode = Marshal.GetLastPInvokeError(); - } - } - finally - { - systemCallErrors = - Marshal.EndTrackingSystemCallErrors(getSystemCallErrors: myHandle != null && myHandle.IsInvalid); - } + // To allow users to view & edit the ACL's, call OpenMutex + // with parameters to allow us to view & edit the ACL. This will + // fail if we don't have permission to view or edit the ACL's. + // If that happens, ask for less permissions. + SafeWaitHandle myHandle = Interop.Kernel32.OpenMutex(AccessRights, false, name); if (myHandle.IsInvalid) { + int errorCode = Marshal.GetLastPInvokeError(); + myHandle.Dispose(); -#if TARGET_UNIX || TARGET_BROWSER || TARGET_WASI - if (errorCode == Interop.Errors.ERROR_FILENAME_EXCED_RANGE) - { - // On Unix, length validation is done by CoreCLR's PAL after converting to utf-8 - throw new ArgumentException(SR.Argument_WaitHandleNameTooLong, nameof(name)); - } -#endif if (Interop.Errors.ERROR_FILE_NOT_FOUND == errorCode || Interop.Errors.ERROR_INVALID_NAME == errorCode) return OpenExistingResult.NameNotFound; if (Interop.Errors.ERROR_PATH_NOT_FOUND == errorCode) @@ -97,7 +58,7 @@ private static OpenExistingResult OpenExistingWorker(string name, out Mutex? res if (Interop.Errors.ERROR_INVALID_HANDLE == errorCode) return OpenExistingResult.NameInvalid; - throw GetExceptionForWin32ErrorWithSystemCallError(errorCode, name, systemCallErrors); + throw Win32Marshal.GetExceptionForWin32Error(errorCode, name); } result = new Mutex(myHandle); @@ -109,41 +70,10 @@ private static OpenExistingResult OpenExistingWorker(string name, out Mutex? res // in a Mutex's ACL is MUTEX_ALL_ACCESS (0x1F0001). public void ReleaseMutex() { - bool success = true; - string? systemCallErrors; - Marshal.BeginTrackingSystemCallErrors(); - try - { - success = Interop.Kernel32.ReleaseMutex(SafeWaitHandle); - } - finally + if (!Interop.Kernel32.ReleaseMutex(SafeWaitHandle)) { - systemCallErrors = Marshal.EndTrackingSystemCallErrors(getSystemCallErrors: !success); + throw new ApplicationException(SR.Arg_SynchronizationLockException); } - - if (!success) - { - SystemException? innerEx = - string.IsNullOrEmpty(systemCallErrors) ? null : GetInnerExceptionForSystemCallErrors(systemCallErrors); - throw new ApplicationException(SR.Arg_SynchronizationLockException, innerEx); - } - } - - private static SystemException GetInnerExceptionForSystemCallErrors(string systemCallErrors) => - new SystemException(SR.Format(SR.SystemException_SystemCallErrors, systemCallErrors)); - - private static Exception GetExceptionForWin32ErrorWithSystemCallError( - int errorCode, - string? path, - string? systemCallErrors) - { - Exception ex = Win32Marshal.GetExceptionForWin32Error(errorCode, path); - if (string.IsNullOrEmpty(systemCallErrors) || ex.GetType() != typeof(IOException)) - { - return ex; - } - - return new IOException(ex.Message, GetInnerExceptionForSystemCallErrors(systemCallErrors)) { HResult = ex.HResult }; } } } diff --git a/src/mono/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Mono.cs index 75f464f75490c8..e2574f415c0de6 100644 --- a/src/mono/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Mono.cs @@ -30,27 +30,6 @@ public partial class Marshal [MethodImplAttribute(MethodImplOptions.InternalCall)] public static extern void SetLastPInvokeError(int error); - /// - /// Begins tracking system call errors on the calling thread during PAL APIs. The system call errors may include - /// information about the system call made, relevant arguments, return values, and error codes. A call to this method - /// should be followed by a call to on the same thread, which returns the set - /// of system call errors that occurred on the thread in that period. Only system call errors that lead to PAL API - /// failures may be tracked. - /// - internal static void BeginTrackingSystemCallErrors() { } - - /// - /// Retrieves system call errors that occurred on the calling thread since - /// was called. - /// - /// Indicates whether to return the accumulated system call errors. - /// - /// System call errors that occurred on the calling thread since was called. - /// Returns null if has not been called, or if no system call - /// errors occurred on the calling thread since it was last called. - /// - internal static string EndTrackingSystemCallErrors(bool getSystemCallErrors) => null; - [RequiresDynamicCode("Marshalling code for the object might not be available. Use the DestroyStructure overload instead.")] [MethodImplAttribute(MethodImplOptions.InternalCall)] [EditorBrowsable(EditorBrowsableState.Never)] From 568e13763eb632ed2751bfda489492caf8acd61f Mon Sep 17 00:00:00 2001 From: Koundinya Veluri Date: Fri, 29 Sep 2023 08:51:30 -0700 Subject: [PATCH 4/7] Undo a couple more things --- src/coreclr/pal/src/include/pal/thread.hpp | 9 +-------- src/coreclr/pal/src/synchmgr/wait.cpp | 1 - src/coreclr/pal/src/thread/thread.cpp | 2 -- 3 files changed, 1 insertion(+), 11 deletions(-) diff --git a/src/coreclr/pal/src/include/pal/thread.hpp b/src/coreclr/pal/src/include/pal/thread.hpp index f9a357eacddc55..86850b260c9301 100644 --- a/src/coreclr/pal/src/include/pal/thread.hpp +++ b/src/coreclr/pal/src/include/pal/thread.hpp @@ -304,10 +304,6 @@ namespace CorUnix // Signal handler's alternate stack to help with stack overflow void* m_alternateStack; - bool m_isTrackingSystemCallErrors; - int m_systemCallErrorsLength; - char *m_systemCallErrors; - // // The thread entry routine (called from InternalCreateThread) // @@ -362,10 +358,7 @@ namespace CorUnix m_fStartStatusSet(FALSE), m_stackBase(NULL), m_stackLimit(NULL), - m_alternateStack(NULL), - m_isTrackingSystemCallErrors(false), - m_systemCallErrorsLength(0), - m_systemCallErrors(NULL) + m_alternateStack(NULL) { }; diff --git a/src/coreclr/pal/src/synchmgr/wait.cpp b/src/coreclr/pal/src/synchmgr/wait.cpp index b44f6abbb7ca18..5ae53759fa2be5 100644 --- a/src/coreclr/pal/src/synchmgr/wait.cpp +++ b/src/coreclr/pal/src/synchmgr/wait.cpp @@ -229,7 +229,6 @@ SignalObjectAndWait( bAlertable ? "TRUE" : "FALSE"); CPalThread *thread = InternalGetCurrentThread(); - DWORD result = InternalSignalObjectAndWait(thread, hObjectToSignal, hObjectToWaitOn, dwMilliseconds, bAlertable); LOGEXIT("SignalObjectAndWait returns DWORD %u\n", result); diff --git a/src/coreclr/pal/src/thread/thread.cpp b/src/coreclr/pal/src/thread/thread.cpp index 9c3fea9ce81a4b..90501bbafbde56 100644 --- a/src/coreclr/pal/src/thread/thread.cpp +++ b/src/coreclr/pal/src/thread/thread.cpp @@ -2258,8 +2258,6 @@ CPalThread::~CPalThread() iError = pthread_mutex_destroy(&m_startMutex); _ASSERTE(0 == iError); } - - delete m_systemCallErrors; } void From 92a5fcb02a2c8e0167d8a2dd24d4e491b2476024 Mon Sep 17 00:00:00 2001 From: Koundinya Veluri Date: Fri, 29 Sep 2023 12:41:30 -0700 Subject: [PATCH 5/7] Address feedback --- .../System.Private.CoreLib.csproj | 4 +- .../System/Threading/Mutex.CoreCLR.Unix.cs | 79 +++++++++++++++- .../System/Threading/Mutex.CoreCLR.Windows.cs | 50 ----------- .../src/System/Threading/Mutex.CoreCLR.cs | 89 ------------------- .../Common/src/System/IO/Win32Marshal.cs | 1 - .../System.Private.CoreLib.Shared.projitems | 2 +- 6 files changed, 80 insertions(+), 145 deletions(-) delete mode 100644 src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Windows.cs delete mode 100644 src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.cs diff --git a/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj index 527e218edd8be0..20073739b2bbd0 100644 --- a/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj +++ b/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj @@ -239,7 +239,6 @@ - @@ -294,11 +293,10 @@ + Common\Interop\Windows\OleAut32\Interop.VariantClear.cs - - diff --git a/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Unix.cs b/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Unix.cs index fa025baab9a143..139a6bfab14d81 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Unix.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Unix.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.IO; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Text; @@ -13,6 +14,82 @@ namespace System.Threading /// public sealed partial class Mutex : WaitHandle { + private const uint AccessRights = + (uint)Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.MUTEX_MODIFY_STATE; + + private void CreateMutexCore(bool initiallyOwned, string? name, out bool createdNew) + { + SafeWaitHandle mutexHandle = + CreateMutexCore(0, initiallyOwned, name, AccessRights, out int errorCode, out string? errorDetails); + + if (mutexHandle.IsInvalid) + { + mutexHandle.SetHandleAsInvalid(); +#if TARGET_UNIX || TARGET_BROWSER || TARGET_WASI + if (errorCode == Interop.Errors.ERROR_FILENAME_EXCED_RANGE) + // On Unix, length validation is done by CoreCLR's PAL after converting to utf-8 + throw new ArgumentException(SR.Argument_WaitHandleNameTooLong, nameof(name)); +#endif + if (errorCode == Interop.Errors.ERROR_INVALID_HANDLE) + throw new WaitHandleCannotBeOpenedException(SR.Format(SR.Threading_WaitHandleCannotBeOpenedException_InvalidHandle, name)); + + throw Win32Marshal.GetExceptionForWin32Error(errorCode, name, errorDetails); + } + + createdNew = errorCode != Interop.Errors.ERROR_ALREADY_EXISTS; + SafeWaitHandle = mutexHandle; + } + + private static OpenExistingResult OpenExistingWorker(string name, out Mutex? result) + { + ArgumentException.ThrowIfNullOrEmpty(name); + + result = null; + // To allow users to view & edit the ACL's, call OpenMutex + // with parameters to allow us to view & edit the ACL. This will + // fail if we don't have permission to view or edit the ACL's. + // If that happens, ask for less permissions. + SafeWaitHandle myHandle = OpenMutexCore(AccessRights, false, name, out int errorCode, out string? errorDetails); + + if (myHandle.IsInvalid) + { + myHandle.Dispose(); + +#if TARGET_UNIX || TARGET_BROWSER || TARGET_WASI + if (errorCode == Interop.Errors.ERROR_FILENAME_EXCED_RANGE) + { + // On Unix, length validation is done by CoreCLR's PAL after converting to utf-8 + throw new ArgumentException(SR.Argument_WaitHandleNameTooLong, nameof(name)); + } +#endif + if (Interop.Errors.ERROR_FILE_NOT_FOUND == errorCode || Interop.Errors.ERROR_INVALID_NAME == errorCode) + return OpenExistingResult.NameNotFound; + if (Interop.Errors.ERROR_PATH_NOT_FOUND == errorCode) + return OpenExistingResult.PathNotFound; + if (Interop.Errors.ERROR_INVALID_HANDLE == errorCode) + return OpenExistingResult.NameInvalid; + + throw Win32Marshal.GetExceptionForWin32Error(errorCode, name, errorDetails); + } + + result = new Mutex(myHandle); + return OpenExistingResult.Success; + } + + // Note: To call ReleaseMutex, you must have an ACL granting you + // MUTEX_MODIFY_STATE rights (0x0001). The other interesting value + // in a Mutex's ACL is MUTEX_ALL_ACCESS (0x1F0001). + public void ReleaseMutex() + { + if (!Interop.Kernel32.ReleaseMutex(SafeWaitHandle)) + { + throw new ApplicationException(SR.Arg_SynchronizationLockException); + } + } + + //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // Unix-specific implementation + private const int SystemCallErrorsBufferSize = 256; private static unsafe SafeWaitHandle CreateMutexCore( @@ -73,7 +150,7 @@ private static unsafe SafeWaitHandle OpenMutexCore( } [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "PAL_CreateMutexW", SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] - private static unsafe partial SafeWaitHandle CreateMutex(nint mutexAttributes, [MarshalAs(UnmanagedType.Bool)] bool initialOwner, string? name, byte *systemCallErrors, uint systemCallErrorsBufferSize); + private static unsafe partial SafeWaitHandle CreateMutex(nint mutexAttributes, [MarshalAs(UnmanagedType.Bool)] bool initialOwner, string? name, byte* systemCallErrors, uint systemCallErrorsBufferSize); [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "PAL_OpenMutexW", SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] private static unsafe partial SafeWaitHandle OpenMutex(uint desiredAccess, [MarshalAs(UnmanagedType.Bool)] bool inheritHandle, string name, byte* systemCallErrors, uint systemCallErrorsBufferSize); diff --git a/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Windows.cs b/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Windows.cs deleted file mode 100644 index 4170a1d8066f7f..00000000000000 --- a/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Windows.cs +++ /dev/null @@ -1,50 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Runtime.InteropServices; -using Microsoft.Win32.SafeHandles; - -namespace System.Threading -{ - /// - /// Synchronization primitive that can also be used for interprocess synchronization - /// - public sealed partial class Mutex : WaitHandle - { - private static SafeWaitHandle CreateMutexCore( - nint mutexAttributes, - bool initialOwner, - string? name, - uint desiredAccess, - out int errorCode, - out string? errorDetails) - { - errorDetails = null; - SafeWaitHandle mutexHandle = - Interop.Kernel32.CreateMutexEx( - mutexAttributes, - name, - initialOwner ? Interop.Kernel32.CREATE_MUTEX_INITIAL_OWNER : 0, - desiredAccess); - - // Get the error code even if the handle is valid, as it could be ERROR_ALREADY_EXISTS, indicating that the mutex - // already exists and was opened - errorCode = Marshal.GetLastPInvokeError(); - - return mutexHandle; - } - - private static SafeWaitHandle OpenMutexCore( - uint desiredAccess, - bool inheritHandle, - string name, - out int errorCode, - out string? errorDetails) - { - errorDetails = null; - SafeWaitHandle mutexHandle = Interop.Kernel32.OpenMutex(desiredAccess, inheritHandle, name); - errorCode = mutexHandle.IsInvalid ? Marshal.GetLastPInvokeError() : Interop.Errors.ERROR_SUCCESS; - return mutexHandle; - } - } -} diff --git a/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.cs deleted file mode 100644 index 9f887724d37a06..00000000000000 --- a/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.cs +++ /dev/null @@ -1,89 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Diagnostics; -using System.IO; -using System.Runtime.InteropServices; -using Microsoft.Win32.SafeHandles; - -namespace System.Threading -{ - /// - /// Synchronization primitive that can also be used for interprocess synchronization - /// - public sealed partial class Mutex : WaitHandle - { - private const uint AccessRights = - (uint)Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.MUTEX_MODIFY_STATE; - - private void CreateMutexCore(bool initiallyOwned, string? name, out bool createdNew) - { - SafeWaitHandle mutexHandle = - CreateMutexCore(0, initiallyOwned, name, AccessRights, out int errorCode, out string? errorDetails); - - if (mutexHandle.IsInvalid) - { - mutexHandle.SetHandleAsInvalid(); -#if TARGET_UNIX || TARGET_BROWSER || TARGET_WASI - if (errorCode == Interop.Errors.ERROR_FILENAME_EXCED_RANGE) - // On Unix, length validation is done by CoreCLR's PAL after converting to utf-8 - throw new ArgumentException(SR.Argument_WaitHandleNameTooLong, nameof(name)); -#endif - if (errorCode == Interop.Errors.ERROR_INVALID_HANDLE) - throw new WaitHandleCannotBeOpenedException(SR.Format(SR.Threading_WaitHandleCannotBeOpenedException_InvalidHandle, name)); - - throw Win32Marshal.GetExceptionForWin32Error(errorCode, name, errorDetails); - } - - createdNew = errorCode != Interop.Errors.ERROR_ALREADY_EXISTS; - SafeWaitHandle = mutexHandle; - } - - private static OpenExistingResult OpenExistingWorker(string name, out Mutex? result) - { - ArgumentException.ThrowIfNullOrEmpty(name); - - result = null; - // To allow users to view & edit the ACL's, call OpenMutex - // with parameters to allow us to view & edit the ACL. This will - // fail if we don't have permission to view or edit the ACL's. - // If that happens, ask for less permissions. - SafeWaitHandle myHandle = OpenMutexCore(AccessRights, false, name, out int errorCode, out string? errorDetails); - - if (myHandle.IsInvalid) - { - myHandle.Dispose(); - -#if TARGET_UNIX || TARGET_BROWSER || TARGET_WASI - if (errorCode == Interop.Errors.ERROR_FILENAME_EXCED_RANGE) - { - // On Unix, length validation is done by CoreCLR's PAL after converting to utf-8 - throw new ArgumentException(SR.Argument_WaitHandleNameTooLong, nameof(name)); - } -#endif - if (Interop.Errors.ERROR_FILE_NOT_FOUND == errorCode || Interop.Errors.ERROR_INVALID_NAME == errorCode) - return OpenExistingResult.NameNotFound; - if (Interop.Errors.ERROR_PATH_NOT_FOUND == errorCode) - return OpenExistingResult.PathNotFound; - if (Interop.Errors.ERROR_INVALID_HANDLE == errorCode) - return OpenExistingResult.NameInvalid; - - throw Win32Marshal.GetExceptionForWin32Error(errorCode, name, errorDetails); - } - - result = new Mutex(myHandle); - return OpenExistingResult.Success; - } - - // Note: To call ReleaseMutex, you must have an ACL granting you - // MUTEX_MODIFY_STATE rights (0x0001). The other interesting value - // in a Mutex's ACL is MUTEX_ALL_ACCESS (0x1F0001). - public void ReleaseMutex() - { - if (!Interop.Kernel32.ReleaseMutex(SafeWaitHandle)) - { - throw new ApplicationException(SR.Arg_SynchronizationLockException); - } - } - } -} diff --git a/src/libraries/Common/src/System/IO/Win32Marshal.cs b/src/libraries/Common/src/System/IO/Win32Marshal.cs index 1c68364b89b86e..0c11b227e677ed 100644 --- a/src/libraries/Common/src/System/IO/Win32Marshal.cs +++ b/src/libraries/Common/src/System/IO/Win32Marshal.cs @@ -3,7 +3,6 @@ using System.Diagnostics; using System.Runtime.InteropServices; -using Microsoft.Win32.SafeHandles; namespace System.IO { diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index 202d51338929b5..e5b218a6b98aae 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -2148,7 +2148,7 @@ - + From bbdde59a6ca9cc6e6ad89723309b211fca01e021 Mon Sep 17 00:00:00 2001 From: Koundinya Veluri Date: Fri, 29 Sep 2023 13:08:08 -0700 Subject: [PATCH 6/7] Remove some Unix ifdefs --- .../src/System/Threading/Mutex.CoreCLR.Unix.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Unix.cs b/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Unix.cs index 139a6bfab14d81..3392ac4b185a49 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Unix.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Unix.cs @@ -25,11 +25,9 @@ private void CreateMutexCore(bool initiallyOwned, string? name, out bool created if (mutexHandle.IsInvalid) { mutexHandle.SetHandleAsInvalid(); -#if TARGET_UNIX || TARGET_BROWSER || TARGET_WASI if (errorCode == Interop.Errors.ERROR_FILENAME_EXCED_RANGE) // On Unix, length validation is done by CoreCLR's PAL after converting to utf-8 throw new ArgumentException(SR.Argument_WaitHandleNameTooLong, nameof(name)); -#endif if (errorCode == Interop.Errors.ERROR_INVALID_HANDLE) throw new WaitHandleCannotBeOpenedException(SR.Format(SR.Threading_WaitHandleCannotBeOpenedException_InvalidHandle, name)); @@ -55,13 +53,11 @@ private static OpenExistingResult OpenExistingWorker(string name, out Mutex? res { myHandle.Dispose(); -#if TARGET_UNIX || TARGET_BROWSER || TARGET_WASI if (errorCode == Interop.Errors.ERROR_FILENAME_EXCED_RANGE) { // On Unix, length validation is done by CoreCLR's PAL after converting to utf-8 throw new ArgumentException(SR.Argument_WaitHandleNameTooLong, nameof(name)); } -#endif if (Interop.Errors.ERROR_FILE_NOT_FOUND == errorCode || Interop.Errors.ERROR_INVALID_NAME == errorCode) return OpenExistingResult.NameNotFound; if (Interop.Errors.ERROR_PATH_NOT_FOUND == errorCode) From 5c37c7cb7aa97827ca4a946d3cf44ab5cce7547c Mon Sep 17 00:00:00 2001 From: Koundinya Veluri Date: Fri, 29 Sep 2023 13:32:13 -0700 Subject: [PATCH 7/7] Address feedback --- .../System/Threading/Mutex.CoreCLR.Unix.cs | 33 ++++--------------- src/coreclr/pal/inc/pal.h | 3 -- src/coreclr/pal/src/synchobj/mutex.cpp | 18 +++------- 3 files changed, 12 insertions(+), 42 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Unix.cs b/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Unix.cs index 3392ac4b185a49..52233ffc583b14 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Unix.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Threading/Mutex.CoreCLR.Unix.cs @@ -14,14 +14,9 @@ namespace System.Threading /// public sealed partial class Mutex : WaitHandle { - private const uint AccessRights = - (uint)Interop.Kernel32.MAXIMUM_ALLOWED | Interop.Kernel32.SYNCHRONIZE | Interop.Kernel32.MUTEX_MODIFY_STATE; - private void CreateMutexCore(bool initiallyOwned, string? name, out bool createdNew) { - SafeWaitHandle mutexHandle = - CreateMutexCore(0, initiallyOwned, name, AccessRights, out int errorCode, out string? errorDetails); - + SafeWaitHandle mutexHandle = CreateMutexCore(initiallyOwned, name, out int errorCode, out string? errorDetails); if (mutexHandle.IsInvalid) { mutexHandle.SetHandleAsInvalid(); @@ -47,7 +42,7 @@ private static OpenExistingResult OpenExistingWorker(string name, out Mutex? res // with parameters to allow us to view & edit the ACL. This will // fail if we don't have permission to view or edit the ACL's. // If that happens, ask for less permissions. - SafeWaitHandle myHandle = OpenMutexCore(AccessRights, false, name, out int errorCode, out string? errorDetails); + SafeWaitHandle myHandle = OpenMutexCore(name, out int errorCode, out string? errorDetails); if (myHandle.IsInvalid) { @@ -89,21 +84,13 @@ public void ReleaseMutex() private const int SystemCallErrorsBufferSize = 256; private static unsafe SafeWaitHandle CreateMutexCore( - nint mutexAttributes, bool initialOwner, string? name, - uint desiredAccess, out int errorCode, out string? errorDetails) { byte* systemCallErrors = stackalloc byte[SystemCallErrorsBufferSize]; - SafeWaitHandle mutexHandle = - CreateMutex( - mutexAttributes, - initialOwner, - name, - systemCallErrors, - SystemCallErrorsBufferSize); + SafeWaitHandle mutexHandle = CreateMutex(initialOwner, name, systemCallErrors, SystemCallErrorsBufferSize); // Get the error code even if the handle is valid, as it could be ERROR_ALREADY_EXISTS, indicating that the mutex // already exists and was opened @@ -113,16 +100,10 @@ private static unsafe SafeWaitHandle CreateMutexCore( return mutexHandle; } - private static unsafe SafeWaitHandle OpenMutexCore( - uint desiredAccess, - bool inheritHandle, - string name, - out int errorCode, - out string? errorDetails) + private static unsafe SafeWaitHandle OpenMutexCore(string name, out int errorCode, out string? errorDetails) { byte* systemCallErrors = stackalloc byte[SystemCallErrorsBufferSize]; - SafeWaitHandle mutexHandle = - OpenMutex(desiredAccess, inheritHandle, name, systemCallErrors, SystemCallErrorsBufferSize); + SafeWaitHandle mutexHandle = OpenMutex(name, systemCallErrors, SystemCallErrorsBufferSize); errorCode = mutexHandle.IsInvalid ? Marshal.GetLastPInvokeError() : Interop.Errors.ERROR_SUCCESS; errorDetails = mutexHandle.IsInvalid ? GetErrorDetails(systemCallErrors) : null; return mutexHandle; @@ -146,9 +127,9 @@ private static unsafe SafeWaitHandle OpenMutexCore( } [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "PAL_CreateMutexW", SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] - private static unsafe partial SafeWaitHandle CreateMutex(nint mutexAttributes, [MarshalAs(UnmanagedType.Bool)] bool initialOwner, string? name, byte* systemCallErrors, uint systemCallErrorsBufferSize); + private static unsafe partial SafeWaitHandle CreateMutex([MarshalAs(UnmanagedType.Bool)] bool initialOwner, string? name, byte* systemCallErrors, uint systemCallErrorsBufferSize); [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "PAL_OpenMutexW", SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] - private static unsafe partial SafeWaitHandle OpenMutex(uint desiredAccess, [MarshalAs(UnmanagedType.Bool)] bool inheritHandle, string name, byte* systemCallErrors, uint systemCallErrorsBufferSize); + private static unsafe partial SafeWaitHandle OpenMutex(string name, byte* systemCallErrors, uint systemCallErrorsBufferSize); } } diff --git a/src/coreclr/pal/inc/pal.h b/src/coreclr/pal/inc/pal.h index d856791237c6e2..189db1ae5b4475 100644 --- a/src/coreclr/pal/inc/pal.h +++ b/src/coreclr/pal/inc/pal.h @@ -1052,7 +1052,6 @@ PALIMPORT HANDLE PALAPI PAL_CreateMutexW( - IN LPSECURITY_ATTRIBUTES lpMutexAttributes, IN BOOL bInitialOwner, IN LPCWSTR lpName, IN LPSTR lpSystemCallErrors, @@ -1075,8 +1074,6 @@ PALIMPORT HANDLE PALAPI PAL_OpenMutexW( - IN DWORD dwDesiredAccess, - IN BOOL bInheritHandle, IN LPCWSTR lpName, IN LPSTR lpSystemCallErrors, IN DWORD dwSystemCallErrorsBufferSize); diff --git a/src/coreclr/pal/src/synchobj/mutex.cpp b/src/coreclr/pal/src/synchobj/mutex.cpp index aacbc648f5d318..e3d63fe232b49f 100644 --- a/src/coreclr/pal/src/synchobj/mutex.cpp +++ b/src/coreclr/pal/src/synchobj/mutex.cpp @@ -103,7 +103,7 @@ CreateMutexW( IN BOOL bInitialOwner, IN LPCWSTR lpName) { - return PAL_CreateMutexW(lpMutexAttributes, bInitialOwner, lpName, nullptr, 0); + return PAL_CreateMutexW(bInitialOwner, lpName, nullptr, 0); } /*++ @@ -125,7 +125,6 @@ CreateMutexW( HANDLE PALAPI PAL_CreateMutexW( - IN LPSECURITY_ATTRIBUTES lpMutexAttributes, IN BOOL bInitialOwner, IN LPCWSTR lpName, IN LPSTR lpSystemCallErrors, @@ -137,9 +136,7 @@ PAL_CreateMutexW( char utf8Name[SHARED_MEMORY_MAX_NAME_CHAR_COUNT + 1]; PERF_ENTRY(PAL_CreateMutexW); - ENTRY("PAL_CreateMutexW(lpMutexAttr=%p, bInitialOwner=%d, lpName=%p (%S), " - "lpSystemCallErrors=%p, dwSystemCallErrorsBufferSize=%d\n", - lpMutexAttributes, + ENTRY("PAL_CreateMutexW(bInitialOwner=%d, lpName=%p (%S), lpSystemCallErrors=%p, dwSystemCallErrorsBufferSize=%d\n", bInitialOwner, lpName, lpName?lpName:W16_NULLSTRING, @@ -185,7 +182,7 @@ PAL_CreateMutexW( palError = InternalCreateMutex( &errors, pthr, - lpMutexAttributes, + nullptr, bInitialOwner, lpName == nullptr ? nullptr : utf8Name, &hMutex @@ -588,7 +585,7 @@ OpenMutexW( IN BOOL bInheritHandle, IN LPCWSTR lpName) { - return PAL_OpenMutexW(dwDesiredAccess, bInheritHandle, lpName, nullptr, 0); + return PAL_OpenMutexW(lpName, nullptr, 0); } /*++ @@ -609,8 +606,6 @@ OpenMutexW( HANDLE PALAPI PAL_OpenMutexW( - IN DWORD dwDesiredAccess, - IN BOOL bInheritHandle, IN LPCWSTR lpName, IN LPSTR lpSystemCallErrors, IN DWORD dwSystemCallErrorsBufferSize) @@ -621,10 +616,7 @@ PAL_OpenMutexW( char utf8Name[SHARED_MEMORY_MAX_NAME_CHAR_COUNT + 1]; PERF_ENTRY(PAL_OpenMutexW); - ENTRY("PAL_OpenMutexW(dwDesiredAccess=%#x, bInheritHandle=%d, lpName=%p (%S), " - "lpSystemCallErrors=%p, dwSystemCallErrorsBufferSize=%d)\n", - dwDesiredAccess, - bInheritHandle, + ENTRY("PAL_OpenMutexW(lpName=%p (%S), lpSystemCallErrors=%p, dwSystemCallErrorsBufferSize=%d)\n", lpName, lpName?lpName:W16_NULLSTRING, lpSystemCallErrors,