diff --git a/src/coreclr/debug/debug-pal/CMakeLists.txt b/src/coreclr/debug/debug-pal/CMakeLists.txt index 475fa62d2c0f1e..59b18e2c91ddb8 100644 --- a/src/coreclr/debug/debug-pal/CMakeLists.txt +++ b/src/coreclr/debug/debug-pal/CMakeLists.txt @@ -1,5 +1,6 @@ include_directories(../inc) include_directories(../../pal/inc) +include_directories(${CLR_SRC_NATIVE_DIR}) add_definitions(-DFEATURE_CORECLR) add_definitions(-DFEATURE_PERFTRACING) @@ -11,16 +12,18 @@ if(CLR_CMAKE_HOST_WIN32) if(CLR_CMAKE_TARGET_WIN32) set(DEBUG_PAL_REFEREENCE_DIAGNOSTICSERVER ON) - set(TWO_WAY_PIPE_SOURCES + set(DEBUG_PAL_SOURCES ${EVENTPIPE_PAL_SOURCES} win/twowaypipe.cpp win/processdescriptor.cpp ) else(CLR_CMAKE_TARGET_WIN32) - set(TWO_WAY_PIPE_SOURCES + set(DEBUG_PAL_SOURCES dummy/twowaypipe.cpp ) endif(CLR_CMAKE_TARGET_WIN32) + + list(APPEND DEBUG_PAL_SOURCES win/wait.cpp) endif(CLR_CMAKE_HOST_WIN32) if(CLR_CMAKE_HOST_UNIX) @@ -36,15 +39,30 @@ if(CLR_CMAKE_HOST_UNIX) include_directories(../../inc) # for volatile.h - set(TWO_WAY_PIPE_SOURCES + set(DEBUG_PAL_SOURCES ${EVENTPIPE_PAL_SOURCES} unix/twowaypipe.cpp unix/processdescriptor.cpp ) + if (NOT CLR_CMAKE_TARGET_ARCH_WASM) + list(APPEND DEBUG_PAL_SOURCES unix/wait.cpp) + # Strict POSIX mode hides pipe2 on BSD even when the platform supports it. + set_property(SOURCE unix/wait.cpp APPEND PROPERTY COMPILE_OPTIONS -U_POSIX_C_SOURCE) + endif() + endif(CLR_CMAKE_HOST_UNIX) -add_library_clr(debug-pal OBJECT ${TWO_WAY_PIPE_SOURCES}) +add_library_clr(debug-pal OBJECT ${DEBUG_PAL_SOURCES}) +target_link_libraries(debug-pal PRIVATE minipal) + +if(CLR_CMAKE_HOST_UNIX) + if(HAVE_PIPE2) + target_compile_definitions(debug-pal PRIVATE HAVE_PIPE2=1) + else() + target_compile_definitions(debug-pal PRIVATE HAVE_PIPE2=0) + endif() +endif(CLR_CMAKE_HOST_UNIX) if (DEBUG_PAL_REFEREENCE_DIAGNOSTICSERVER) target_link_libraries(debug-pal PRIVATE dn-diagnosticserver-pal) diff --git a/src/coreclr/debug/debug-pal/unix/wait.cpp b/src/coreclr/debug/debug-pal/unix/wait.cpp new file mode 100644 index 00000000000000..2829a882bb6b07 --- /dev/null +++ b/src/coreclr/debug/debug-pal/unix/wait.cpp @@ -0,0 +1,535 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include "debugwait.h" + +namespace +{ + constexpr uint32_t MaxWaitHandles = 64; + + enum class AcquireResult + { + Failed, + NotReady, + Ready, + }; + + struct Waitable + { + minipal_mutex mutex; + int32_t refCount; + int readFileDescriptor; + int writeFileDescriptor; + bool mutexInitialized; + bool resetOnWait; + bool signaled; + }; + + void CloseFileDescriptor(int fileDescriptor) + { + if (fileDescriptor >= 0) + { + close(fileDescriptor); + } + } + + bool SetCloseOnExec(int fileDescriptor) + { + int descriptorFlags; + do + { + descriptorFlags = fcntl(fileDescriptor, F_GETFD); + } while (descriptorFlags < 0 && errno == EINTR); + + if (descriptorFlags < 0) + { + return false; + } + + int result; + do + { + result = fcntl(fileDescriptor, F_SETFD, descriptorFlags | FD_CLOEXEC); + } while (result < 0 && errno == EINTR); + + return result == 0; + } + + bool SetPipeFlags(int fileDescriptor) + { + int statusFlags; + do + { + statusFlags = fcntl(fileDescriptor, F_GETFL); + } while (statusFlags < 0 && errno == EINTR); + + if (statusFlags < 0) + { + return false; + } + + int result; + do + { + result = fcntl(fileDescriptor, F_SETFL, statusFlags | O_NONBLOCK); + } while (result < 0 && errno == EINTR); + + return result == 0 && SetCloseOnExec(fileDescriptor); + } + + bool CreatePipe(int fileDescriptors[2]) + { +#if HAVE_PIPE2 + int result; + do + { + result = pipe2(fileDescriptors, O_CLOEXEC | O_NONBLOCK); + } while (result < 0 && errno == EINTR); + + if (result == 0) + { + return true; + } + + if (errno != ENOSYS && errno != EINVAL) + { + return false; + } +#else + int result; +#endif // HAVE_PIPE2 + do + { + result = pipe(fileDescriptors); + } while (result < 0 && errno == EINTR); + + if (result < 0) + { + return false; + } + + if (!SetPipeFlags(fileDescriptors[0]) || + !SetPipeFlags(fileDescriptors[1])) + { + CloseFileDescriptor(fileDescriptors[0]); + CloseFileDescriptor(fileDescriptors[1]); + return false; + } + + return true; + } + + Waitable* AllocateWaitable(bool resetOnWait) + { + Waitable* waitable = new (std::nothrow) Waitable; + if (waitable == nullptr) + { + return nullptr; + } + + waitable->refCount = 1; + waitable->mutexInitialized = false; + waitable->resetOnWait = resetOnWait; + waitable->signaled = false; + waitable->readFileDescriptor = -1; + waitable->writeFileDescriptor = -1; + + if (!minipal_mutex_init(&waitable->mutex)) + { + delete waitable; + return nullptr; + } + + waitable->mutexInitialized = true; + return waitable; + } + + void DestroyWaitable(Waitable* waitable) + { + assert(__atomic_load_n(&waitable->refCount, __ATOMIC_RELAXED) == 0); + + CloseFileDescriptor(waitable->readFileDescriptor); + CloseFileDescriptor(waitable->writeFileDescriptor); + + if (waitable->mutexInitialized) + { + minipal_mutex_destroy(&waitable->mutex); + } + + delete waitable; + } + + void AddReference(Waitable* waitable) + { + __atomic_add_fetch(&waitable->refCount, 1, __ATOMIC_RELAXED); + } + + void ReleaseReference(Waitable* waitable) + { + if (__atomic_sub_fetch(&waitable->refCount, 1, __ATOMIC_ACQ_REL) == 0) + { + DestroyWaitable(waitable); + } + } + + bool WriteByte(int fileDescriptor) + { + uint8_t value = 1; + ssize_t result; + do + { + result = write(fileDescriptor, &value, sizeof(value)); + } while (result < 0 && errno == EINTR); + + return result == sizeof(value); + } + + bool ReadByte(int fileDescriptor) + { + uint8_t value; + ssize_t result; + do + { + result = read(fileDescriptor, &value, sizeof(value)); + } while (result < 0 && errno == EINTR); + + return result == sizeof(value); + } + + bool SignalEventLocked(Waitable* waitable) + { + if (waitable->signaled) + { + return true; + } + + if (!WriteByte(waitable->writeFileDescriptor)) + { + return false; + } + + waitable->signaled = true; + return true; + } + + bool SignalEvent(Waitable* waitable) + { + minipal::MutexHolder lock(waitable->mutex); + return SignalEventLocked(waitable); + } + + bool ResetEvent(Waitable* waitable) + { + assert(waitable->resetOnWait); + minipal::MutexHolder lock(waitable->mutex); + if (!waitable->signaled) + { + return true; + } + + uint8_t buffer[16]; + ssize_t result; + do + { + result = read(waitable->readFileDescriptor, buffer, sizeof(buffer)); + } while (result > 0 || (result < 0 && errno == EINTR)); + + if (result < 0 && errno != EAGAIN) + { + return false; + } + + waitable->signaled = false; + return true; + } + + AcquireResult TryAcquire(Waitable* waitable) + { + minipal::MutexHolder lock(waitable->mutex); + if (!waitable->signaled) + { + return AcquireResult::NotReady; + } + + if (waitable->resetOnWait && + !ReadByte(waitable->readFileDescriptor)) + { + return AcquireResult::Failed; + } + + if (waitable->resetOnWait) + { + waitable->signaled = false; + } + return AcquireResult::Ready; + } + + int32_t TryAcquireAny(Waitable* const* waitables, uint32_t count) + { + for (uint32_t index = 0; index < count; index++) + { + AcquireResult result = TryAcquire(waitables[index]); + if (result == AcquireResult::Failed) + { + return WaitHandle::Failed; + } + + if (result == AcquireResult::Ready) + { + return static_cast(index); + } + } + + return WaitHandle::Timeout; + } + + bool GetMonotonicTime(uint64_t* nanoseconds) + { + timespec currentTime; + if (clock_gettime(CLOCK_MONOTONIC, ¤tTime) != 0) + { + return false; + } + + *nanoseconds = + static_cast(currentTime.tv_sec) * 1000000000 + + static_cast(currentTime.tv_nsec); + return true; + } + + bool GetRemainingNanoseconds(uint64_t deadline, uint64_t* remaining) + { + uint64_t currentTime; + if (!GetMonotonicTime(¤tTime)) + { + return false; + } + + *remaining = currentTime >= deadline ? 0 : deadline - currentTime; + return true; + } + + int GetPollTimeout(uint64_t remainingNanoseconds) + { + uint64_t remainingMilliseconds = (remainingNanoseconds + 999999) / 1000000; + return remainingMilliseconds > INT_MAX + ? INT_MAX + : static_cast(remainingMilliseconds); + } + + Waitable* CreateWaitable(bool initialState, bool resetOnWait) + { + Waitable* waitable = AllocateWaitable(resetOnWait); + if (waitable == nullptr) + { + return nullptr; + } + + int eventPipe[2]; + if (!CreatePipe(eventPipe)) + { + ReleaseReference(waitable); + return nullptr; + } + + waitable->readFileDescriptor = eventPipe[0]; + waitable->writeFileDescriptor = eventPipe[1]; + + if (initialState && !SignalEvent(waitable)) + { + ReleaseReference(waitable); + return nullptr; + } + + return waitable; + } + + int32_t WaitWithPoll( + Waitable* const* waitables, + uint32_t count, + uint32_t timeout, + uint64_t deadline) + { + pollfd descriptors[MaxWaitHandles] = {}; + for (uint32_t index = 0; index < count; index++) + { + descriptors[index].fd = waitables[index]->readFileDescriptor; + descriptors[index].events = POLLIN; + } + + while (true) + { + int32_t acquired = TryAcquireAny(waitables, count); + if (acquired != WaitHandle::Timeout || timeout == 0) + { + return acquired; + } + + int pollTimeout = -1; + if (timeout != WaitHandle::Infinite) + { + uint64_t remaining; + if (!GetRemainingNanoseconds(deadline, &remaining)) + { + return WaitHandle::Failed; + } + + if (remaining == 0) + { + return WaitHandle::Timeout; + } + + pollTimeout = GetPollTimeout(remaining); + } + + int result = poll(descriptors, count, pollTimeout); + if (result == 0) + { + return WaitHandle::Timeout; + } + + if (result < 0) + { + if (errno == EINTR) + { + continue; + } + + return WaitHandle::Failed; + } + + for (uint32_t index = 0; index < count; index++) + { + if ((descriptors[index].revents & POLLNVAL) != 0) + { + errno = EBADF; + return WaitHandle::Failed; + } + } + } + } + + void* DuplicateWaitable(void* handle) + { + if (handle == nullptr) + { + errno = EINVAL; + return nullptr; + } + + Waitable* waitable = static_cast(handle); + AddReference(waitable); + return waitable; + } +} + +WaitHandle::WaitHandle(void* handle) + : m_handle(handle) +{ +} + +WaitHandle::WaitHandle(const WaitHandle& handle) + : WaitHandle(DuplicateWaitable(handle.m_handle)) +{ +} + +WaitHandle::~WaitHandle() +{ + if (m_handle != nullptr) + { + ReleaseReference(static_cast(m_handle)); + } +} + +WaitEvent::WaitEvent(bool initialState) + : WaitHandle(CreateWaitable(initialState, true)) +{ +} + +bool WaitEvent::Set() +{ + if (!IsValid()) + { + errno = EINVAL; + return false; + } + + return SignalEvent(static_cast(GetWaitable())); +} + +bool WaitEvent::Reset() +{ + if (!IsValid()) + { + errno = EINVAL; + return false; + } + + return ResetEvent(static_cast(GetWaitable())); +} + +WaitLatch::WaitLatch() + : WaitHandle(CreateWaitable(false, false)) +{ +} + +bool WaitLatch::Set() +{ + if (!IsValid()) + { + errno = EINVAL; + return false; + } + + return SignalEvent(static_cast(GetWaitable())); +} + +int32_t WaitHandle::Wait( + const WaitHandle* const* handles, + uint32_t count, + uint32_t timeout) +{ + if (handles == nullptr || count == 0 || count > MaxWaitHandles) + { + errno = EINVAL; + return Failed; + } + + Waitable* waitables[MaxWaitHandles]; + for (uint32_t index = 0; index < count; index++) + { + if (handles[index] == nullptr || !handles[index]->IsValid()) + { + errno = EINVAL; + return Failed; + } + + waitables[index] = static_cast(handles[index]->m_handle); + } + + uint64_t deadline = 0; + if (timeout != 0 && timeout != Infinite) + { + uint64_t currentTime; + if (!GetMonotonicTime(¤tTime)) + { + return Failed; + } + + deadline = currentTime + static_cast(timeout) * 1000000; + } + + return WaitWithPoll(waitables, count, timeout, deadline); +} diff --git a/src/coreclr/debug/debug-pal/win/wait.cpp b/src/coreclr/debug/debug-pal/win/wait.cpp new file mode 100644 index 00000000000000..1f3d88de9b2852 --- /dev/null +++ b/src/coreclr/debug/debug-pal/win/wait.cpp @@ -0,0 +1,131 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#include + +#include "debugwait.h" + +namespace +{ + constexpr uint32_t MaxWaitHandles = 64; + + HANDLE DuplicateNativeHandle(HANDLE handle) + { + if (handle == nullptr) + { + SetLastError(ERROR_INVALID_HANDLE); + return nullptr; + } + + HANDLE duplicate = nullptr; + if (!DuplicateHandle( + GetCurrentProcess(), + handle, + GetCurrentProcess(), + &duplicate, + 0, + FALSE, + DUPLICATE_SAME_ACCESS)) + { + return nullptr; + } + + return duplicate; + } +} + +WaitHandle::WaitHandle(HANDLE handle) + : m_handle(handle) +{ +} + +WaitHandle::WaitHandle(const WaitHandle& handle) + : WaitHandle(DuplicateNativeHandle(handle.m_handle)) +{ +} + +WaitHandle::~WaitHandle() +{ + if (m_handle != nullptr) + { + CloseHandle(m_handle); + } +} + +WaitEvent::WaitEvent(bool initialState) + : WaitHandle(CreateEventW(nullptr, FALSE, initialState, nullptr)) +{ +} + +WaitEvent::WaitEvent(HANDLE handle) + : WaitHandle(DuplicateNativeHandle(handle)) +{ +} + +bool WaitEvent::Set() +{ + return SetEvent(GetRawHandle()) != FALSE; +} + +bool WaitEvent::Reset() +{ + return ResetEvent(GetRawHandle()) != FALSE; +} + +WaitLatch::WaitLatch() + : WaitHandle(CreateEventW(nullptr, TRUE, FALSE, nullptr)) +{ +} + +bool WaitLatch::Set() +{ + return SetEvent(GetRawHandle()) != FALSE; +} + +NativeHandle::NativeHandle(HANDLE handle) + : WaitHandle(DuplicateNativeHandle(handle)) +{ +} + +int32_t WaitHandle::Wait( + const WaitHandle* const* handles, + uint32_t count, + uint32_t timeout) +{ + if (handles == nullptr || count == 0 || count > MaxWaitHandles) + { + SetLastError(ERROR_INVALID_PARAMETER); + return Failed; + } + + HANDLE nativeHandles[MaxWaitHandles]; + for (uint32_t index = 0; index < count; index++) + { + if (handles[index] == nullptr || !handles[index]->IsValid()) + { + SetLastError(ERROR_INVALID_HANDLE); + return Failed; + } + + nativeHandles[index] = handles[index]->m_handle; + } + + DWORD result = WaitForMultipleObjectsEx( + count, + nativeHandles, + FALSE, + timeout, + FALSE); + + if (result >= WAIT_OBJECT_0 && result < WAIT_OBJECT_0 + count) + { + return static_cast(result - WAIT_OBJECT_0); + } + + if (result == WAIT_TIMEOUT) + { + return Timeout; + } + + return Failed; +} diff --git a/src/coreclr/debug/di/dbgtransportmanager.cpp b/src/coreclr/debug/di/dbgtransportmanager.cpp index de3ee71b093944..52141464515c30 100644 --- a/src/coreclr/debug/di/dbgtransportmanager.cpp +++ b/src/coreclr/debug/di/dbgtransportmanager.cpp @@ -12,7 +12,7 @@ #include #include #include -#endif +#endif // HOST_UNIX DbgTransportTarget g_DbgTransportTarget{}; @@ -20,9 +20,8 @@ DbgTransportTarget g_DbgTransportTarget{}; // Polling interval for the per-process exit poller thread. static const useconds_t s_processExitPollIntervalUsec = 250 * 1000; -// Polls the target PID for exit. Uses waitpid(WNOHANG) for child processes -// (immune to PID reuse) and falls back to kill(pid, 0) for non-children -// (best-effort, racy under PID reuse). Signals m_hProcessExited on exit. +// Polls the target PID for exit. waitpid is identity-stable for child processes; kill is a +// best-effort fallback for non-children and can race PID reuse. /* static */ void *DbgTransportTarget::ProcessExitPollerThread(void *arg) { @@ -41,27 +40,29 @@ void *DbgTransportTarget::ProcessExitPollerThread(void *arg) if (r == (pid_t)entry->m_dwPID) { - exited = true; + exited = WIFEXITED(status) || WIFSIGNALED(status); } else if (r == -1 && errno == ECHILD) { - // Not our child; fall back to kill(pid, 0). - if (kill(entry->m_dwPID, 0) != 0 && errno == ESRCH) + int killResult; + do { - exited = true; - } + killResult = kill(entry->m_dwPID, 0); + } while (killResult == -1 && errno == EINTR); + + exited = killResult == -1 && errno == ESRCH; } if (exited) { - SetEvent(entry->m_hProcessExited); + entry->m_hProcessExited->Set(); break; } usleep(s_processExitPollIntervalUsec); } - return NULL; + return nullptr; } #endif // HOST_UNIX @@ -104,7 +105,7 @@ void DbgTransportTarget::Shutdown() // on for process termination. HRESULT DbgTransportTarget::GetTransportForProcess(const ProcessDescriptor *pProcessDescriptor, DbgTransportSession **ppTransport, - HANDLE *phProcessHandle) + WaitHandle **ppProcessHandle) { RSLockHolder lock(&m_sLock); HRESULT hr = S_OK; @@ -126,9 +127,9 @@ HRESULT DbgTransportTarget::GetTransportForProcess(const ProcessDescriptor *pPr } - // Probe the process to make sure it exists, then create a waitable handle that becomes - // signaled on process exit. On HOST_WINDOWS the process handle itself is waitable; on - // HOST_UNIX we create a manual-reset event and start a thread to poll for exit. + // Probe the process to make sure it exists, then create a waitable that becomes signaled + // on process exit. Windows uses the process handle itself; Unix uses a latch set by + // a poller thread. #ifdef HOST_UNIX if (kill(dwPID, 0) != 0) { @@ -136,43 +137,53 @@ HRESULT DbgTransportTarget::GetTransportForProcess(const ProcessDescriptor *pPr return (errno == ESRCH) ? E_INVALIDARG : E_FAIL; } - HANDLE hProcessExited = CreateEvent(NULL, TRUE, FALSE, NULL); - if (hProcessExited == NULL) + WaitLatch *pProcessExited = new (nothrow) WaitLatch(); + if ((pProcessExited == NULL) || !pProcessExited->IsValid()) { + delete pProcessExited; transport->Shutdown(); - return HRESULT_FROM_GetLastError(); + return E_FAIL; } #else // HOST_UNIX - HANDLE hProcessExited = OpenProcess(PROCESS_ALL_ACCESS, FALSE, dwPID); - if (hProcessExited == NULL) + HANDLE hProcess = OpenProcess(PROCESS_ALL_ACCESS, FALSE, dwPID); + if (hProcess == NULL) { transport->Shutdown(); return HRESULT_FROM_GetLastError(); } + + NativeHandle *pProcessExited = new (nothrow) NativeHandle(hProcess); + bool allocationFailed = pProcessExited == nullptr; + DWORD error = GetLastError(); + CloseHandle(hProcess); + if (allocationFailed || !pProcessExited->IsValid()) + { + delete pProcessExited; + transport->Shutdown(); + return allocationFailed ? E_OUTOFMEMORY : HRESULT_FROM_WIN32(error); + } #endif // HOST_UNIX newEntry->m_dwPID = dwPID; - newEntry->m_hProcessExited = hProcessExited; + newEntry->m_hProcessExited = pProcessExited; #ifdef HOST_UNIX newEntry->m_fStopPoller = false; newEntry->m_fPollerStarted = false; - if (pthread_create(&newEntry->m_pollerThread, NULL, &ProcessExitPollerThread, newEntry.GetValue()) != 0) + if (pthread_create(&newEntry->m_pollerThread, nullptr, &ProcessExitPollerThread, newEntry.GetValue()) != 0) { transport->Shutdown(); - CloseHandle(hProcessExited); - newEntry->m_hProcessExited = NULL; return E_FAIL; } newEntry->m_fPollerStarted = true; #endif // HOST_UNIX // Initialize it (this immediately starts the remote connection process). - hr = transport->Init(*pProcessDescriptor, hProcessExited); + hr = transport->Init(*pProcessDescriptor, *pProcessExited); if (FAILED(hr)) { transport->Shutdown(); - // ProcessEntry destructor stops the poller thread and closes the event handle. + // ProcessEntry destructor stops the poller and releases the waitable. return hr; } @@ -190,18 +201,15 @@ HRESULT DbgTransportTarget::GetTransportForProcess(const ProcessDescriptor *pPr entry->m_cProcessRef++; _ASSERTE(entry->m_cProcessRef > 0); _ASSERTE(entry->m_transport != NULL); - _ASSERTE((intptr_t)entry->m_hProcessExited > 0); + _ASSERTE(entry->m_hProcessExited->IsValid()); *ppTransport = entry->m_transport; - if (!DuplicateHandle(GetCurrentProcess(), - entry->m_hProcessExited, - GetCurrentProcess(), - phProcessHandle, - 0, // ignored since we are going to pass DUPLICATE_SAME_ACCESS - FALSE, - DUPLICATE_SAME_ACCESS)) + *ppProcessHandle = new (nothrow) WaitHandle(*entry->m_hProcessExited); + if ((*ppProcessHandle == nullptr) || !(*ppProcessHandle)->IsValid()) { - return HRESULT_FROM_GetLastError(); + delete *ppProcessHandle; + *ppProcessHandle = nullptr; + return E_FAIL; } return hr; @@ -227,7 +235,7 @@ void DbgTransportTarget::ReleaseTransport(DbgTransportSession *pTransport) _ASSERTE(entry->m_cProcessRef > 0); _ASSERTE(entry->m_transport != NULL); - _ASSERTE((intptr_t)entry->m_hProcessExited > 0); + _ASSERTE(entry->m_hProcessExited->IsValid()); if (entry->m_transport == pTransport) { @@ -271,14 +279,14 @@ DbgTransportTarget::ProcessEntry::~ProcessEntry() if (m_fPollerStarted) { m_fStopPoller = true; - pthread_join(m_pollerThread, NULL); + pthread_join(m_pollerThread, nullptr); m_fPollerStarted = false; } -#endif +#endif // HOST_UNIX if (m_hProcessExited != NULL) { - CloseHandle(m_hProcessExited); + delete m_hProcessExited; m_hProcessExited = NULL; } diff --git a/src/coreclr/debug/di/dbgtransportmanager.h b/src/coreclr/debug/di/dbgtransportmanager.h index 76e150ccbfb7ef..114b2246aa58d5 100644 --- a/src/coreclr/debug/di/dbgtransportmanager.h +++ b/src/coreclr/debug/di/dbgtransportmanager.h @@ -9,7 +9,9 @@ #ifdef HOST_UNIX #include -#endif +#endif // HOST_UNIX + +#include "debugwait.h" // TODO: Ideally we'd like to remove this class and don't do any process related book keeping in DBI. @@ -39,7 +41,10 @@ class DbgTransportTarget // Given a PID attempt to find or create a DbgTransportSession instance to manage a connection to a // runtime in that process. Returns E_UNEXPECTED if the process can't be found. Also returns a handle that // can be waited on for process termination. - HRESULT GetTransportForProcess(const ProcessDescriptor *pProcessDescriptor, DbgTransportSession **ppTransport, HANDLE *phProcessHandle); + HRESULT GetTransportForProcess( + const ProcessDescriptor *pProcessDescriptor, + DbgTransportSession **ppTransport, + WaitHandle **ppProcessHandle); // Give back a previously acquired transport (if nobody else is using the transport it will close down the // connection at this point). @@ -56,10 +61,11 @@ class DbgTransportTarget { ProcessEntry *m_pNext; // Next entry in the list DWORD m_dwPID; // Process ID for this entry - HANDLE m_hProcessExited; // Waitable handle that becomes signaled when the - // process exits. On HOST_WINDOWS this is the process - // handle itself; on HOST_UNIX it is a manual-reset - // event signaled by the poller thread below. +#ifdef HOST_UNIX + WaitLatch *m_hProcessExited; // Latch set when the process exits +#else + NativeHandle *m_hProcessExited; // Native process handle +#endif // HOST_UNIX DbgTransportSession *m_transport; // Debugger's connection to the process DWORD m_cProcessRef; // Ref count #ifdef HOST_UNIX @@ -73,7 +79,7 @@ class DbgTransportTarget #ifdef HOST_UNIX static void *ProcessExitPollerThread(void *arg); -#endif +#endif // HOST_UNIX ProcessEntry *m_pProcessList; // Head of list of currently alive processes (unsorted) RSLock m_sLock; // Lock protecting read and write access to the target list diff --git a/src/coreclr/debug/di/dbgtransportpipeline.cpp b/src/coreclr/debug/di/dbgtransportpipeline.cpp index b42f7c4534e080..ddb64cf78a3683 100644 --- a/src/coreclr/debug/di/dbgtransportpipeline.cpp +++ b/src/coreclr/debug/di/dbgtransportpipeline.cpp @@ -57,11 +57,12 @@ class DbgTransportPipeline : public: DbgTransportPipeline() { - m_fRunning = FALSE; - m_hProcess = NULL; - m_pIPCEvent = reinterpret_cast(m_rgbIPCEventBuffer); - m_pProxy = NULL; - m_pTransport = NULL; + m_fRunning = FALSE; + m_fProcessExitEventPending = FALSE; + m_hProcess = NULL; + m_pIPCEvent = reinterpret_cast(m_rgbIPCEventBuffer); + m_pProxy = NULL; + m_pTransport = NULL; _ASSERTE(!IsTransportRunning()); } @@ -91,7 +92,7 @@ class DbgTransportPipeline : ); // Return a handle which will be signaled when the debuggee process terminates. - virtual HANDLE GetProcessHandle(); + virtual WaitHandle *GetProcessHandle(); // Terminate the debuggee process. virtual BOOL TerminateProcess(UINT32 exitCode); @@ -113,9 +114,11 @@ class DbgTransportPipeline : // clean up all resources void Dispose() { + m_fProcessExitEventPending = FALSE; + if (m_hProcess != NULL) { - CloseHandle(m_hProcess); + delete m_hProcess; } m_hProcess = NULL; @@ -132,10 +135,11 @@ class DbgTransportPipeline : } BOOL m_fRunning; + BOOL m_fProcessExitEventPending; DWORD m_dwProcessId; - // This is actually a handle to an event. This is only valid for waiting on process termination. - HANDLE m_hProcess; + // This waitable is only valid for waiting on process termination. + WaitHandle *m_hProcess; DbgTransportTarget * m_pProxy; DbgTransportSession * m_pTransport; @@ -200,6 +204,7 @@ HRESULT DbgTransportPipeline::DebugActiveProcess(MachineInfo machineInfo, const if (SUCCEEDED(hr)) { m_dwProcessId = processDescriptor.m_Pid; + m_fProcessExitEventPending = FALSE; m_fRunning = TRUE; } else @@ -229,14 +234,14 @@ BOOL DbgTransportPipeline::WaitForDebugEvent(DEBUG_EVENT * pEvent, DWORD dwTimeo // We need to wait for a debug event from the transport and the process termination event. // On Windows, process termination is communicated via a debug event as well, but that's not true for // the Mac debugging transport. - DWORD cWaitSet = 2; - HANDLE rghWaitSet[2]; - rghWaitSet[0] = m_pTransport->GetDebugEventReadyEvent(); - rghWaitSet[1] = m_hProcess; + const WaitHandle *waitSet[] = { + m_pTransport->GetDebugEventReadyEvent(), + m_hProcess + }; - DWORD dwRet = ::WaitForMultipleObjectsEx(cWaitSet, rghWaitSet, FALSE, dwTimeout, FALSE); + int32_t waitResult = WaitHandle::Wait(waitSet, ARRAY_SIZE(waitSet), dwTimeout); - if (dwRet == WAIT_OBJECT_0) + if (waitResult == 0) { // The Mac debugging transport actually transmits IPC events and not debug events. // We need to convert the IPC event to a debug event and pass it back to the caller. @@ -256,7 +261,7 @@ BOOL DbgTransportPipeline::WaitForDebugEvent(DEBUG_EVENT * pEvent, DWORD dwTimeo return TRUE; } - else if (dwRet == (WAIT_OBJECT_0 + 1)) + else if (waitResult == 1) { // The process has been terminated. @@ -266,9 +271,8 @@ BOOL DbgTransportPipeline::WaitForDebugEvent(DEBUG_EVENT * pEvent, DWORD dwTimeo pEvent->dwThreadId = 0; // On Windows this is the first thread created in the process. pEvent->u.ExitProcess.dwExitCode = 0; // This is not passed back to us by the transport. - // Once the process termination event is signaled, we cannot send or receive any events. - // So we mark the transport as not running anymore. - m_fRunning = FALSE; + // The shim will continue this synthesized event before asking for another one. + m_fProcessExitEventPending = TRUE; return TRUE; } else @@ -285,6 +289,13 @@ BOOL DbgTransportPipeline::ContinueDebugEvent( DWORD dwContinueStatus ) { + if (m_fProcessExitEventPending) + { + m_fProcessExitEventPending = FALSE; + m_fRunning = FALSE; + return TRUE; + } + if (!IsTransportRunning()) { return FALSE; @@ -295,24 +306,23 @@ BOOL DbgTransportPipeline::ContinueDebugEvent( } // Return a handle which will be signaled when the debuggee process terminates. -HANDLE DbgTransportPipeline::GetProcessHandle() +WaitHandle *DbgTransportPipeline::GetProcessHandle() { - HANDLE hProcessTerminated; - - if (!DuplicateHandle(GetCurrentProcess(), - m_hProcess, - GetCurrentProcess(), - &hProcessTerminated, - 0, // ignored since we are going to pass DUPLICATE_SAME_ACCESS - FALSE, - DUPLICATE_SAME_ACCESS)) + // The handle returned here is only valid for waiting on process termination. + // See code:INativeEventPipeline::GetProcessHandle. + if (m_hProcess == nullptr) { - return NULL; + return nullptr; } - // The handle returned here is only valid for waiting on process termination. - // See code:INativeEventPipeline::GetProcessHandle. - return hProcessTerminated; + WaitHandle *processHandle = new (nothrow) WaitHandle(*m_hProcess); + if ((processHandle != nullptr) && !processHandle->IsValid()) + { + delete processHandle; + processHandle = nullptr; + } + + return processHandle; } // Terminate the debuggee process. diff --git a/src/coreclr/debug/di/eventchannel.h b/src/coreclr/debug/di/eventchannel.h index 0376a85010275a..9163a36c48e4f7 100644 --- a/src/coreclr/debug/di/eventchannel.h +++ b/src/coreclr/debug/di/eventchannel.h @@ -12,6 +12,8 @@ #ifndef _EVENT_CHANNEL_H_ #define _EVENT_CHANNEL_H_ +#include "debugwait.h" + //--------------------------------------------------------------------------------------- // // This is the abstract base class for the old-style "IPC" event channel. (Despite the name, these events are @@ -154,13 +156,13 @@ class IEventChannel // first to see if it is necessary to wait for an acknowledgement. // // Return Value: - // a handle to a Win32 event which will be signaled when the LS acknowledges the receipt of the IPC event + // a waitable event which will be signaled when the LS acknowledges the receipt of the IPC event // // Assumptions: // NeedToWaitForAck() returns true after sending an IPC event to the LS // - virtual HANDLE GetRightSideEventAckHandle() = 0; + virtual WaitEvent *GetRightSideEventAckHandle() = 0; // // After sending an event to the LS and determining that we need to wait for the LS's acknowledgement, diff --git a/src/coreclr/debug/di/localeventchannel.cpp b/src/coreclr/debug/di/localeventchannel.cpp index a7eb5fd9a5c405..6145f6cab76ff2 100644 --- a/src/coreclr/debug/di/localeventchannel.cpp +++ b/src/coreclr/debug/di/localeventchannel.cpp @@ -51,7 +51,7 @@ class LocalEventChannel : public IEventChannel virtual BOOL NeedToWaitForAck(DebuggerIPCEvent * pEvent); // Get a handle to wait on after sending an IPC event to the LS. The caller should call NeedToWaitForAck() - virtual HANDLE GetRightSideEventAckHandle(); + virtual WaitEvent *GetRightSideEventAckHandle(); // Clean up the state if the wait for an acknowledgement is unsuccessful. virtual void ClearEventForLeftSide(); @@ -100,6 +100,7 @@ class LocalEventChannel : public IEventChannel // used by the LS to signal that the event is read HANDLE m_rightSideEventRead; + WaitEvent *m_rightSideEventReadWaitHandle; // handle of the debuggee process HANDLE m_hTargetProc; @@ -159,6 +160,7 @@ LocalEventChannel::LocalEventChannel(CORDB_ADDRESS pLeftSideDCB, m_rightSideEventAvailable = NULL; m_rightSideEventRead = NULL; + m_rightSideEventReadWaitHandle = nullptr; m_pMutableDataTarget.Assign(pMutableDataTarget); } @@ -190,6 +192,19 @@ HRESULT LocalEventChannel::Init(HANDLE hTargetProc) IfFailRet(DuplicateHandleToLocalProcess(&m_rightSideEventRead, &m_pDCBBuffer->m_rightSideEventRead)); + m_rightSideEventReadWaitHandle = new (nothrow) WaitEvent(m_rightSideEventRead); + if (m_rightSideEventReadWaitHandle == nullptr) + { + return E_OUTOFMEMORY; + } + if (!m_rightSideEventReadWaitHandle->IsValid()) + { + HRESULT hr = HRESULT_FROM_GetLastError(); + delete m_rightSideEventReadWaitHandle; + m_rightSideEventReadWaitHandle = nullptr; + return hr; + } + return S_OK; } @@ -234,6 +249,9 @@ void LocalEventChannel::Delete() m_rightSideEventRead = NULL; } + delete m_rightSideEventReadWaitHandle; + m_rightSideEventReadWaitHandle = nullptr; + if (m_pDCBBuffer != NULL) { delete m_pDCBBuffer; @@ -294,9 +312,9 @@ BOOL LocalEventChannel::NeedToWaitForAck(DebuggerIPCEvent * pEvent) // Get a handle to wait on after sending an IPC event to the LS. The caller should call NeedToWaitForAck() // // virtual -HANDLE LocalEventChannel::GetRightSideEventAckHandle() +WaitEvent *LocalEventChannel::GetRightSideEventAckHandle() { - return m_rightSideEventRead; + return m_rightSideEventReadWaitHandle; } // Clean up the state if the wait for an acknowledgement is unsuccessful. diff --git a/src/coreclr/debug/di/nativepipeline.h b/src/coreclr/debug/di/nativepipeline.h index 442ab32138b5ff..516e172aee557b 100644 --- a/src/coreclr/debug/di/nativepipeline.h +++ b/src/coreclr/debug/di/nativepipeline.h @@ -109,12 +109,12 @@ class INativeEventPipeline // handle for the debuggee process (see below) // // Notes: - // Handles are a Windows-specific concept. For Mac debugging, the handle returned by this function is - // only valid for waiting on process termination. This is ok for now because the only cases where a - // real process handle is needed are related to interop-debugging, which isn't supported on the Mac. + // Handles are a Windows-specific concept. On Unix, the returned value is a debug-pal latch that is only + // valid for debugger-internal process-termination waits. It must not be exposed through + // ICorDebugProcess::GetHandle. // - virtual HANDLE GetProcessHandle() = 0; + virtual WaitHandle *GetProcessHandle() = 0; // // Terminate the debuggee process. @@ -175,4 +175,3 @@ BOOL IsExceptionEvent(const DEBUG_EVENT * pEvent, BOOL * pfFirstChance, const EX INativeEventPipeline * NewPipelineForThisPlatform(); #endif // _NATIVE_PIPELINE_H - diff --git a/src/coreclr/debug/di/process.cpp b/src/coreclr/debug/di/process.cpp index b7464de024d85d..21ddf8b5f37383 100644 --- a/src/coreclr/debug/di/process.cpp +++ b/src/coreclr/debug/di/process.cpp @@ -960,8 +960,9 @@ CordbProcess::CordbProcess(ULONG64 clrInstanceId, CordbHashTable m_steppers; // Closed in ~CordbProcess + // Deleted in ~CordbProcess + WaitEvent *m_leftSideEventAvailable; // Closed in CloseIPCEventHandles called from ~CordbProcess - HANDLE m_leftSideEventAvailable; HANDLE m_leftSideEventRead; // Closed in ~CordbProcess @@ -988,6 +989,11 @@ CordbProcess::~CordbProcess() // We shouldn't still be in Cordb's list of processes. Unfortunately, our root Cordb object // may have already been deleted b/c we're at the mercy of ref-counting, so we can't check. + // RCET wait sets hold non-owning pointers to this wrapper while retaining the CordbProcess. + // Keep it alive until the final process reference is released. + delete m_leftSideEventAvailable; + m_leftSideEventAvailable = NULL; + m_processMutex.Destroy(); m_StopGoLock.Destroy(); @@ -1067,14 +1073,15 @@ HRESULT ShimProcess::DebugActiveProcess( // being 'managed attached' if(!pShim->m_fIsInteropDebugging) { - DWORD dwHandles = 2; - HANDLE arrHandles[2]; - - arrHandles[0] = pShim->m_terminatingEvent; - arrHandles[1] = pShim->m_markAttachPendingEvent; + WaitEvent terminatingEvent(pShim->m_terminatingEvent); + WaitEvent markAttachPendingEvent(pShim->m_markAttachPendingEvent); + const WaitHandle *waitSet[] = { &terminatingEvent, &markAttachPendingEvent }; // Wait for the completion of marking pending attach bit or debugger detaching - WaitForMultipleObjectsEx(dwHandles, arrHandles, FALSE, INFINITE, FALSE); + WaitHandle::Wait( + waitSet, + ARRAY_SIZE(waitSet), + WaitHandle::Infinite); } #endif //!FEATURE_DBGIPC_TRANSPORT_DI } @@ -1269,13 +1276,6 @@ void CordbProcess::CloseIPCHandles() { INTERNAL_API_ENTRY(this); - // Close off Right Side's handles. - if (m_leftSideEventAvailable != NULL) - { - CloseHandle(m_leftSideEventAvailable); - m_leftSideEventAvailable = NULL; - } - if (m_leftSideEventRead != NULL) { CloseHandle(m_leftSideEventRead); @@ -1284,12 +1284,7 @@ void CordbProcess::CloseIPCHandles() if (m_handle != NULL) { - // @dbgtodo - We should probably add asserts to all calls to CloseHandles(), but this has been - // a particularly problematic spot in the past for Mac debugging. - BOOL fSuccess = CloseHandle(m_handle); - (void)fSuccess; //prevent "unused variable" error from GCC - _ASSERTE(fSuccess); - + delete m_handle; m_handle = NULL; } @@ -1625,10 +1620,12 @@ HRESULT CordbProcess::Init() // signal existing RS infrastructure. Eventually get rid of LSEA, LSER completely. // - m_leftSideEventAvailable = CreateEvent(NULL, FALSE, FALSE, NULL); - if (m_leftSideEventAvailable == NULL) + m_leftSideEventAvailable = new (nothrow) WaitEvent(false); + if ((m_leftSideEventAvailable == nullptr) || !m_leftSideEventAvailable->IsValid()) { - ThrowLastError(); + delete m_leftSideEventAvailable; + m_leftSideEventAvailable = nullptr; + ThrowOutOfMemory(); } m_leftSideEventRead = CreateEvent(NULL, FALSE, FALSE, NULL); @@ -1657,7 +1654,7 @@ HRESULT CordbProcess::Init() // This is not needed in the V3 pipeline because we don't assume we have a live, local, process. m_handle = GetShim()->GetNativePipeline()->GetProcessHandle(); - if (m_handle == NULL) + if ((m_handle == nullptr) || !m_handle->IsValid()) { ThrowLastError(); } @@ -1780,7 +1777,7 @@ void CordbProcess::Terminating(BOOL fDetach) // But don't set RSER unless we actually read the event. We don't block on RSER // since that wait also checks the leftside's process handle. SetEvent(m_leftSideEventRead); - SetEvent(m_leftSideEventAvailable); + m_leftSideEventAvailable->Set(); SetEvent(m_stopWaitEvent); if (m_pShim != NULL) @@ -3016,9 +3013,10 @@ void CordbProcess::DetachShim() IfFailThrow(hr); #ifdef OUT_OF_PROCESS_SETTHREADCONTEXT - const HANDLE rghWaitSet[] = { - m_detachSetThreadContextNeededEvent, // Signaled on every debug event after the first SendCanDetach request - UnsafeGetProcessHandle() // Signaled when the process exits + WaitEvent detachSetThreadContextNeededEvent(m_detachSetThreadContextNeededEvent); + const WaitHandle *waitSet[] = { + &detachSetThreadContextNeededEvent, // Signaled on every debug event after the first SendCanDetach request + UnsafeGetProcessWaitHandle() // Signaled when the process exits }; bool fDetachComplete = !m_fOutOfProcessSetThreadContextEventReceived; @@ -3029,14 +3027,17 @@ void CordbProcess::DetachShim() } while (!fDetachComplete) { - DWORD dwResult = WaitForMultipleObjectsEx(_countof(rghWaitSet), rghWaitSet, FALSE, DETACH_WAIT_TIMEOUT_MS, FALSE); - if (dwResult == WAIT_OBJECT_0) + int32_t waitResult = WaitHandle::Wait( + waitSet, + ARRAY_SIZE(waitSet), + DETACH_WAIT_TIMEOUT_MS); + if (waitResult == 0) { // We have been signaled via TryDetach() to determine if it is safe to detach // so call CanDetach and then detach if it returns S_OK fDetachComplete = (this->m_pShim->GetWin32EventThread()->SendCanDetach() == S_OK); } - else if (dwResult == WAIT_OBJECT_0 + 1 /*UnsafeGetProcessHandle()*/) + else if (waitResult == 1) { // The process has exited while waiting for the detach to complete m_detached = true; @@ -3048,7 +3049,7 @@ void CordbProcess::DetachShim() // We timed out waiting for debug events, indicating the process is idle. // Simply detach as if it had succeeded. - _ASSERTE(dwResult == WAIT_TIMEOUT); + _ASSERTE(waitResult == WaitHandle::Timeout); CONSISTENCY_CHECK_MSGF(false, ("Timeout while waiting for detach to complete")); fDetachComplete = true; @@ -3282,6 +3283,12 @@ HRESULT CordbProcess::GetHandle(HANDLE *phProcessHandle) FAIL_IF_NEUTERED(this); // Once we neuter the process, we close our OS handle to it. VALIDATE_POINTER_TO_OBJECT(phProcessHandle, HANDLE *); +#ifdef HOST_UNIX + // COMPAT: Unix callers historically received an opaque PAL process handle and only rely on this + // API succeeding, so return the process ID as an opaque sentinel now that DBI uses minipal internally. + *phProcessHandle = reinterpret_cast(static_cast(GetProcessDescriptor()->m_Pid)); + return S_OK; +#else if (m_pShim == NULL) { _ASSERTE(!"CordbProcess::GetHandle() should be not be called on the new architecture"); @@ -3290,9 +3297,10 @@ HRESULT CordbProcess::GetHandle(HANDLE *phProcessHandle) } else { - *phProcessHandle = m_handle; + *phProcessHandle = UnsafeGetProcessHandle(); return S_OK; } +#endif } HRESULT CordbProcess::IsRunning(BOOL *pbRunning) @@ -7936,7 +7944,7 @@ HRESULT CordbProcess::StartSyncFromWin32Stop(BOOL * pfAsyncBreakSent) bool CordbProcess::CheckIfLSExited() { // Check by waiting on the handle with no timeout. - if (WaitForSingleObject(m_handle, 0) == WAIT_OBJECT_0) + if (WaitHandle::Wait(*m_handle, 0) == 0) { Lock(); m_terminated = true; @@ -8748,7 +8756,7 @@ CordbRCEventThread::CordbRCEventThread(Cordb* cordb) CordbRCEventThread::~CordbRCEventThread() { if (m_threadControlEvent != NULL) - CloseHandle(m_threadControlEvent); + delete m_threadControlEvent; if (m_thread != NULL) CloseHandle(m_thread); @@ -8764,10 +8772,14 @@ HRESULT CordbRCEventThread::Init() if (m_cordb == NULL) return E_INVALIDARG; - m_threadControlEvent = CreateEvent(NULL, FALSE, FALSE, NULL); + m_threadControlEvent = new (nothrow) WaitEvent(false); - if (m_threadControlEvent == NULL) - return HRESULT_FROM_GetLastError(); + if ((m_threadControlEvent == nullptr) || !m_threadControlEvent->IsValid()) + { + delete m_threadControlEvent; + m_threadControlEvent = nullptr; + return E_OUTOFMEMORY; + } return S_OK; } @@ -8791,7 +8803,7 @@ void CordbProcess::DuplicateHandleToLocalProcess(HANDLE * pLocalHandle, RemoteHA // On Launch, we don't have them yet, but on attach we do. if (*pLocalHandle == NULL) { - BOOL fSuccess = pRemoteHandle->DuplicateToLocalProcess(m_handle, pLocalHandle); + BOOL fSuccess = pRemoteHandle->DuplicateToLocalProcess(UnsafeGetProcessHandle(), pLocalHandle); if (!fSuccess) { ThrowLastError(); @@ -8851,7 +8863,7 @@ void CordbProcess::FinishInitializeIPCChannelWorker() LOG((LF_CORDB, LL_EVERYTHING, "Size of CdbP is %zu\n", sizeof(CordbProcess))); - m_pEventChannel->Init(m_handle); + IfFailThrow(m_pEventChannel->Init(UnsafeGetProcessHandle())); #if defined(FEATURE_INTEROP_DEBUGGING) DuplicateHandleToLocalProcess(&m_leftSideUnmanagedWaitEvent, &GetDCB()->m_leftSideUnmanagedWaitEvent); @@ -9327,13 +9339,11 @@ HRESULT CordbRCEventThread::SendIPCEvent(CordbProcess* process, } else { - // Get a handle to the target process - this call always succeeds - HANDLE hLSProcess = NULL; - process->GetHandle(&hLSProcess); + WaitHandle *pLSProcess = process->UnsafeGetProcessWaitHandle(); // We take locks to ensure that the CordbProcess object is still alive, // even if the OS process exited. - _ASSERTE(hLSProcess != NULL); + _ASSERTE(pLSProcess != nullptr); // Check if Sending the IPC event failed if (FAILED(hr)) @@ -9343,8 +9353,8 @@ HRESULT CordbRCEventThread::SendIPCEvent(CordbProcess* process, // There is a race here - we can't rely on any check above SendEventToLeftSide // to tell us whether the process has exited yet. // Check for that case and return an accurate hresult. - DWORD ret = WaitForSingleObject(hLSProcess, 0); - if (ret == WAIT_OBJECT_0) + int32_t waitResult = WaitHandle::Wait(*pLSProcess, 0); + if (waitResult == 0) { return CORDBG_E_PROCESS_TERMINATED; } @@ -9362,7 +9372,7 @@ HRESULT CordbRCEventThread::SendIPCEvent(CordbProcess* process, { STRESS_LOG0(LF_CORDB, LL_INFO1000,"CRCET::SIPCE: waiting for left side to read event. (on RSER)\n"); - DWORD ret; + int32_t waitResult; // Wait for either a reply (common case) or the left side to go away. // We can't detach while waiting for a reply (because detach needs to send events). @@ -9371,7 +9381,7 @@ HRESULT CordbRCEventThread::SendIPCEvent(CordbProcess* process, // and so ExitProcess may have been called, but it doesn't matter. enum { - ID_RSER = WAIT_OBJECT_0, + ID_RSER = 0, ID_LSPROCESS, ID_HELPERTHREAD, }; @@ -9381,29 +9391,48 @@ HRESULT CordbRCEventThread::SendIPCEvent(CordbProcess* process, // follow up with an exit. // This includes when we've dispatch Native events, and it includes the AsyncBreak sent to get us from a // win32 frozen state to a synchronized state). +#ifdef HOST_WINDOWS + NativeHandle *pHelperThreadWait = nullptr; HANDLE hHelperThread = NULL; if (process->IsStopped()) { hHelperThread = process->GetHelperThreadHandle(); } + if (hHelperThread != NULL) + { + pHelperThreadWait = new (nothrow) NativeHandle(hHelperThread); + if ((pHelperThreadWait == nullptr) || !pHelperThreadWait->IsValid()) + { + delete pHelperThreadWait; + return E_OUTOFMEMORY; + } + } +#else + WaitHandle *pHelperThreadWait = nullptr; +#endif - // Note that in case of a tie (multiple handles signaled), WaitForMultipleObjects gives - // priority to the handle earlier in the array. - HANDLE waitSet[] = { process->GetEventChannel()->GetRightSideEventAckHandle(), hLSProcess, hHelperThread}; + // In a tie, wait-any gives priority to the handle earlier in the array. + const WaitHandle *waitSet[] = { + process->GetEventChannel()->GetRightSideEventAckHandle(), + pLSProcess, + pHelperThreadWait + }; DWORD cWaitSet = ARRAY_SIZE(waitSet); - if (hHelperThread == NULL) + if (pHelperThreadWait == nullptr) { cWaitSet--; } do { - ret = WaitForMultipleObjectsEx(cWaitSet, waitSet, FALSE, CordbGetWaitTimeout(), FALSE); + waitResult = WaitHandle::Wait(waitSet, cWaitSet, CordbGetWaitTimeout()); // If we timeout because we're waiting for an uncontinued OOB event, we need to just keep waiting. - } while ((ret == WAIT_TIMEOUT) && process->IsWaitingForOOBEvent()); + } while ((waitResult == WaitHandle::Timeout) && process->IsWaitingForOOBEvent()); - switch(ret) + delete pHelperThreadWait; + + switch(waitResult) { case ID_RSER: // Normal reply from LS. @@ -9456,7 +9485,7 @@ HRESULT CordbRCEventThread::SendIPCEvent(CordbProcess* process, // If we timed out/failed, check the left side to see if it is in the unrecoverable error mode. If it is, // return the HR from the left side that caused the error. Otherwise, return that we timed out and that // we don't really know why. - HRESULT realHR = (ret == WAIT_FAILED) ? HRESULT_FROM_GetLastError() : ErrWrapper(CORDBG_E_TIMEOUT); + HRESULT realHR = waitResult == WaitHandle::Failed ? E_FAIL : ErrWrapper(CORDBG_E_TIMEOUT); hr = process->CheckForUnrecoverableError(); @@ -9663,7 +9692,7 @@ void CordbRCEventThread::ProcessStateChanged() m_cordb->LockProcessList(); STRESS_LOG0(LF_CORDB, LL_INFO100000, "CRCET::ProcessStateChanged\n"); m_processStateChanged = TRUE; - SetEvent(m_threadControlEvent); + m_threadControlEvent->Set(); m_cordb->UnlockProcessList(); } @@ -9683,13 +9712,13 @@ void CordbRCEventThread::ProcessStateChanged() //--------------------------------------------------------------------------------------- void CordbRCEventThread::ThreadProc() { - HANDLE waitSet[MAXIMUM_WAIT_OBJECTS]; + const WaitHandle *waitSet[MAXIMUM_WAIT_OBJECTS]; CordbProcess * rgProcessSet[MAXIMUM_WAIT_OBJECTS]; unsigned int waitCount; #ifdef _DEBUG memset(&rgProcessSet, 0, MAXIMUM_WAIT_OBJECTS * sizeof(CordbProcess *)); - memset(&waitSet, 0, MAXIMUM_WAIT_OBJECTS * sizeof(HANDLE)); + memset(&waitSet, 0, sizeof(waitSet)); #endif @@ -9700,18 +9729,17 @@ void CordbRCEventThread::ThreadProc() while (m_run) { - DWORD dwStatus = WaitForMultipleObjectsEx(waitCount, waitSet, FALSE, 2000, FALSE); + int32_t waitResult = WaitHandle::Wait(waitSet, waitCount, 2000); - if (dwStatus == WAIT_FAILED) + if (waitResult == WaitHandle::Failed) { - STRESS_LOG1(LF_CORDB, LL_INFO10000, "CordbRCEventThread::ThreadProc WaitFor" - "MultipleObjects failed: 0x%x\n", GetLastError()); + STRESS_LOG0(LF_CORDB, LL_INFO10000, "CordbRCEventThread::ThreadProc wait failed\n"); } #ifdef _DEBUG - else if ((dwStatus >= WAIT_OBJECT_0) && (dwStatus < WAIT_OBJECT_0 + waitCount) && m_run) + else if ((waitResult >= 0) && (static_cast(waitResult) < waitCount) && m_run) { // Got an event. Figure out which process it came from. - unsigned int procNumber = dwStatus - WAIT_OBJECT_0; + unsigned int procNumber = static_cast(waitResult); if (procNumber != 0) { @@ -9965,7 +9993,7 @@ void CordbRCEventThread::QueueAsyncWorkItem(RCETWorkItem * pItem) m_WorkerStack.Push(pItem); // Ping the RCET so that it drains the queue. - SetEvent(m_threadControlEvent); + m_threadControlEvent->Set(); } // Execute & delete all workitems in the queue. @@ -10020,26 +10048,27 @@ HRESULT CordbRCEventThread::WaitForIPCEventFromProcess(CordbProcess * pProcess, CORDBRequireProcessStateOKAndSync(pProcess, pAppDomain); - DWORD dwStatus; + int32_t waitResult; HRESULT hr = S_OK; do { - dwStatus = SafeWaitForSingleObject(pProcess, - pProcess->m_leftSideEventAvailable, - CordbGetWaitTimeout()); + _ASSERTE(!pProcess->ThreadHoldsProcessLock()); + waitResult = WaitHandle::Wait( + *pProcess->m_leftSideEventAvailable, + CordbGetWaitTimeout()); if (pProcess->m_terminated) { return CORDBG_E_PROCESS_TERMINATED; } // If we timeout because we're waiting for an uncontinued OOB event, we need to just keep waiting. - } while ((dwStatus == WAIT_TIMEOUT) && pProcess->IsWaitingForOOBEvent()); + } while ((waitResult == WaitHandle::Timeout) && pProcess->IsWaitingForOOBEvent()); - if (dwStatus == WAIT_OBJECT_0) + if (waitResult == 0) { pProcess->CopyRCEventFromIPCBlock(pEvent); @@ -10060,7 +10089,7 @@ HRESULT CordbRCEventThread::WaitForIPCEventFromProcess(CordbProcess * pProcess, return hr; } - else if (dwStatus == WAIT_TIMEOUT) + else if (waitResult == WaitHandle::Timeout) { // // If we timed out, check the left side to see if it is in the @@ -10082,9 +10111,9 @@ HRESULT CordbRCEventThread::WaitForIPCEventFromProcess(CordbProcess * pProcess, } else { - _ASSERTE(dwStatus == WAIT_FAILED); + _ASSERTE(waitResult == WaitHandle::Failed); - hr = HRESULT_FROM_GetLastError(); + hr = E_FAIL; CORDBSetUnrecoverableError(pProcess, hr, 0); @@ -10131,7 +10160,7 @@ HRESULT CordbRCEventThread::Stop() m_run = FALSE; - SetEvent(m_threadControlEvent); + m_threadControlEvent->Set(); DWORD ret = WaitForSingleObject(m_thread, INFINITE); @@ -10200,7 +10229,7 @@ CordbWin32EventThread::~CordbWin32EventThread() CloseHandle(m_thread); if (m_threadControlEvent != NULL) - CloseHandle(m_threadControlEvent); + delete m_threadControlEvent; if (m_actionTakenEvent != NULL) CloseHandle(m_actionTakenEvent); @@ -10225,9 +10254,13 @@ HRESULT CordbWin32EventThread::Init() m_sendToWin32EventThreadMutex.Init("Win32-Send lock", RSLock::cLockFlat, RSLock::LL_WIN32_SEND_LOCK); - m_threadControlEvent = CreateEvent(NULL, FALSE, FALSE, NULL); - if (m_threadControlEvent == NULL) - return HRESULT_FROM_GetLastError(); + m_threadControlEvent = new (nothrow) WaitEvent(false); + if ((m_threadControlEvent == nullptr) || !m_threadControlEvent->IsValid()) + { + delete m_threadControlEvent; + m_threadControlEvent = nullptr; + return E_OUTOFMEMORY; + } m_actionTakenEvent = CreateEvent(NULL, FALSE, FALSE, NULL); if (m_actionTakenEvent == NULL) @@ -10337,7 +10370,7 @@ void CordbProcess::FilterClrNotification( // Save the IPC event and wake up the thread which is waiting for it from the LS. GetEventChannel()->SaveEventFromLeftSide(pManagedEvent); - SetEvent(this->m_leftSideEventAvailable); + this->m_leftSideEventAvailable->Set(); // Some other thread called code:CordbRCEventThread::WaitForIPCEventFromProcess, and // that will respond here and set the event. @@ -11163,7 +11196,7 @@ void CordbWin32EventThread::Win32EventLoop() // Have to wait on 2 sources: - // WaitForMultipleObjects - ping for messages (create, attach, Continue, detach) and also + // Control wait set - ping for messages (create, attach, Continue, detach) and also // process exits in the managed-only case. // Native Debug Events - This is a huge perf hit so we want to avoid it whenever we can. // Only wait on these if we're interop debugging and if the process is not frozen. @@ -11172,9 +11205,9 @@ void CordbWin32EventThread::Win32EventLoop() unsigned int cWaitCount = 1; - HANDLE rghWaitSet[2]; + const WaitHandle *waitSet[2]; - rghWaitSet[0] = m_threadControlEvent; + waitSet[0] = m_threadControlEvent; DWORD dwWaitTimeout = INFINITE; DEBUG_EVENT event = {}; @@ -11237,16 +11270,17 @@ void CordbWin32EventThread::Win32EventLoop() // that will ensure that we only wait for an Exit event once. if ((m_pProcess != NULL) && fDidNotJustGetExitProcessEvent) { - rghWaitSet[1] = m_pProcess->UnsafeGetProcessHandle(); + waitSet[1] = m_pProcess->UnsafeGetProcessWaitHandle(); cWaitCount = 2; } // See if any process that we aren't attached to as the Win32 debugger have exited. (Note: this is a // polling action if we are also waiting for Win32 debugger events. We're also looking at the thread // control event here, too, to see if we're supposed to do something, like attach. - DWORD dwStatus = WaitForMultipleObjectsEx(cWaitCount, rghWaitSet, FALSE, dwWaitTimeout, FALSE); + int32_t waitResult = WaitHandle::Wait(waitSet, cWaitCount, dwWaitTimeout); - _ASSERTE((dwStatus == WAIT_TIMEOUT) || (dwStatus < cWaitCount)); + _ASSERTE((waitResult == WaitHandle::Timeout) || + (waitResult >= 0 && static_cast(waitResult) < cWaitCount)); if (!m_run) { @@ -11255,15 +11289,15 @@ void CordbWin32EventThread::Win32EventLoop() } LOG((LF_CORDB, LL_INFO100000, "W32ET::W32EL - got event , ret=%d, has w32 dbg event=%d\n", - dwStatus, fEventAvailable)); + waitResult, fEventAvailable)); // If we haven't timed out, or if it wasn't the thread control event // that was set, then a process has // exited... - if ((dwStatus != WAIT_TIMEOUT) && (dwStatus != WAIT_OBJECT_0)) + if ((waitResult != WaitHandle::Timeout) && (waitResult != 0)) { // Grab the process that exited. - _ASSERTE((dwStatus - WAIT_OBJECT_0) == 1); + _ASSERTE(waitResult == 1); ExitProcess(false); // not detach fEventAvailable = false; } @@ -11517,7 +11551,7 @@ CordbUnmanagedThread * CordbProcess::GetUnmanagedThreadFromEvent(const DEBUG_EVE // Kill the process. // RS will pump events until we LS process exits. - TerminateProcess(this->m_handle, hr); + TerminateProcess(UnsafeGetProcessHandle(), hr); return pUnmanagedThread; } @@ -12147,7 +12181,7 @@ Reaction CordbProcess::TriageExcep1stChanceAndInit(CordbUnmanagedThread * pUnman this->m_helperThreadDead = true; // This only works on Windows, not on Mac. We don't support interop-debugging on Mac anyway. - SetEvent(m_pEventChannel->GetRightSideEventAckHandle()); + m_pEventChannel->GetRightSideEventAckHandle()->Set(); // Note: we remember that this was a second chance event from one of the special stack overflow // cases with CUES_ExceptionUnclearable. This tells us to force the process to terminate when we @@ -13439,7 +13473,7 @@ HRESULT CordbWin32EventThread::SendDebugActiveProcessEvent( // threads from making requests at the same time. m_action = W32ETA_ATTACH_PROCESS; - BOOL succ = SetEvent(m_threadControlEvent); + BOOL succ = m_threadControlEvent->Set(); if (succ) { @@ -13678,7 +13712,7 @@ HRESULT CordbWin32EventThread::SendDetachProcessEvent(CordbProcess *pProcess) // requests at the same time. m_action = W32ETA_DETACH; - BOOL succ = SetEvent(m_threadControlEvent); + BOOL succ = m_threadControlEvent->Set(); if (succ) { @@ -13725,7 +13759,7 @@ HRESULT CordbWin32EventThread::SendUnmanagedContinue(CordbProcess *pProcess, // threads from making requests at the same time. m_action = W32ETA_CONTINUE; - BOOL succ = SetEvent(m_threadControlEvent); + BOOL succ = m_threadControlEvent->Set(); if (succ) { @@ -14190,7 +14224,7 @@ HRESULT CordbWin32EventThread::SendCanDetach() m_action = W32ETA_CAN_DETACH; - BOOL succ = SetEvent(m_threadControlEvent); + BOOL succ = m_threadControlEvent->Set(); if (succ) { @@ -14269,7 +14303,7 @@ HRESULT CordbWin32EventThread::Stop() m_action = W32ETA_NONE; m_run = FALSE; - SetEvent(m_threadControlEvent); + m_threadControlEvent->Set(); UnlockSendToWin32EventThreadMutex(); DWORD ret = WaitForSingleObject(m_thread, INFINITE); diff --git a/src/coreclr/debug/di/remoteeventchannel.cpp b/src/coreclr/debug/di/remoteeventchannel.cpp index 47a104720e5c1d..23d0302f9eff34 100644 --- a/src/coreclr/debug/di/remoteeventchannel.cpp +++ b/src/coreclr/debug/di/remoteeventchannel.cpp @@ -52,7 +52,7 @@ class RemoteEventChannel : public IEventChannel virtual BOOL NeedToWaitForAck(DebuggerIPCEvent * pEvent); // Get a handle to wait on after sending an IPC event to the LS. The caller should call NeedToWaitForAck() - virtual HANDLE GetRightSideEventAckHandle(); + virtual WaitEvent *GetRightSideEventAckHandle(); // Clean up the state if the wait for an acknowledgement is unsuccessful. virtual void ClearEventForLeftSide(); @@ -95,7 +95,7 @@ HRESULT NewEventChannelForThisPlatform(CORDB_ADDRESS pLeftSideDCB, { // @dbgtodo Mac - Consider moving all of the transport logic to one place. // Perhaps add a new function on DbgTransportManager. - HandleHolder hDummy; + WaitHandle *processWaitHandle = nullptr; HRESULT hr = E_FAIL; RemoteEventChannel * pEventChannel = NULL; @@ -104,7 +104,7 @@ HRESULT NewEventChannelForThisPlatform(CORDB_ADDRESS pLeftSideDCB, DbgTransportTarget * pProxy = &g_DbgTransportTarget; DbgTransportSession * pTransport = NULL; - hr = pProxy->GetTransportForProcess(pProcessDescriptor, &pTransport, &hDummy); + hr = pProxy->GetTransportForProcess(pProcessDescriptor, &pTransport, &processWaitHandle); if (FAILED(hr)) { goto Label_Exit; @@ -134,6 +134,11 @@ HRESULT NewEventChannelForThisPlatform(CORDB_ADDRESS pLeftSideDCB, *ppEventChannel = pEventChannel; Label_Exit: + if (processWaitHandle != nullptr) + { + delete processWaitHandle; + } + if (FAILED(hr)) { if (pEventChannel != NULL) @@ -266,7 +271,7 @@ BOOL RemoteEventChannel::NeedToWaitForAck(DebuggerIPCEvent * pEvent) // Get a handle to wait on after sending an IPC event to the LS. The caller should call NeedToWaitForAck() // // virtual -HANDLE RemoteEventChannel::GetRightSideEventAckHandle() +WaitEvent *RemoteEventChannel::GetRightSideEventAckHandle() { // Delegate to the transport which does the real work. return m_pTransport->GetIPCEventReadyEvent(); diff --git a/src/coreclr/debug/di/rspriv.h b/src/coreclr/debug/di/rspriv.h index fae880461662cd..f8011e14d564a8 100644 --- a/src/coreclr/debug/di/rspriv.h +++ b/src/coreclr/debug/di/rspriv.h @@ -15,6 +15,7 @@ #include #include +#include "debugwait.h" #include @@ -3692,20 +3693,25 @@ class CordbProcess : RSSmartPtr m_cordb; private: - // OS process handle to live process. - // @dbgtodo - , Move this into the Shim. This should only be needed in the live-process - // case. Get rid of this since it breaks the data-target abstraction. - // For Mac debugging, this handle is of course not the real process handle. This is just a handle to - // wait on for process termination. - HANDLE m_handle; + // Process-exit waitable. On Windows this wraps an OS process handle. On Unix it is a debug-pal latch + // that is valid only with the debug-pal wait APIs and must not be exposed to clients. + WaitHandle *m_handle; // Process descriptor - holds PID and App group ID for Mac debugging ProcessDescriptor m_processDescriptor; public: - // Wrapper to get the OS process handle. This is unsafe because it breaks the data-target abstraction. - // The only things that need this should be calls to DuplicateHandle, and some shimming work. - HANDLE UnsafeGetProcessHandle() + // Windows-only callers may also use the returned value as the native process handle. + HANDLE UnsafeGetProcessHandle() + { +#ifdef HOST_WINDOWS + return m_handle == nullptr ? NULL : m_handle->GetRawHandle(); +#else + return NULL; +#endif + } + + WaitHandle *UnsafeGetProcessWaitHandle() { return m_handle; } @@ -3868,7 +3874,7 @@ class CordbProcess : DebuggerIPCRuntimeOffsets m_runtimeOffsets; - HANDLE m_leftSideEventAvailable; + WaitEvent *m_leftSideEventAvailable; HANDLE m_leftSideEventRead; #if defined(FEATURE_INTEROP_DEBUGGING) HANDLE m_leftSideUnmanagedWaitEvent; @@ -10121,7 +10127,7 @@ class CordbWin32EventThread HANDLE m_thread; DWORD m_threadId; - HANDLE m_threadControlEvent; + WaitEvent *m_threadControlEvent; HANDLE m_actionTakenEvent; BOOL m_run; @@ -10295,7 +10301,7 @@ class CordbRCEventThread HANDLE m_thread; DWORD m_threadId; BOOL m_run; - HANDLE m_threadControlEvent; + WaitEvent *m_threadControlEvent; BOOL m_processStateChanged; }; diff --git a/src/coreclr/debug/di/shimremotedatatarget.cpp b/src/coreclr/debug/di/shimremotedatatarget.cpp index de3413e1c7fc1b..2690de4a5b3a42 100644 --- a/src/coreclr/debug/di/shimremotedatatarget.cpp +++ b/src/coreclr/debug/di/shimremotedatatarget.cpp @@ -190,14 +190,14 @@ HRESULT BuildPlatformSpecificDataTarget(MachineInfo machineInfo, const ProcessDescriptor * pProcessDescriptor, ShimDataTarget ** ppDataTarget) { - HandleHolder hDummy; + WaitHandle *processWaitHandle = nullptr; HRESULT hr = E_FAIL; ShimRemoteDataTarget * pRemoteDataTarget = NULL; DbgTransportTarget * pProxy = &g_DbgTransportTarget; DbgTransportSession * pTransport = NULL; - hr = pProxy->GetTransportForProcess(pProcessDescriptor, &pTransport, &hDummy); + hr = pProxy->GetTransportForProcess(pProcessDescriptor, &pTransport, &processWaitHandle); if (FAILED(hr)) { goto Label_Exit; @@ -221,6 +221,8 @@ HRESULT BuildPlatformSpecificDataTarget(MachineInfo machineInfo, pRemoteDataTarget->AddRef(); // must addref out-parameters Label_Exit: + delete processWaitHandle; + if (FAILED(hr)) { if (pRemoteDataTarget != NULL) diff --git a/src/coreclr/debug/di/windowspipeline.cpp b/src/coreclr/debug/di/windowspipeline.cpp index 9ed41ee7e1da76..cfe81085b3b737 100644 --- a/src/coreclr/debug/di/windowspipeline.cpp +++ b/src/coreclr/debug/di/windowspipeline.cpp @@ -71,7 +71,7 @@ class WindowsNativePipeline : ); // Return a handle for the debuggee process. - virtual HANDLE GetProcessHandle(); + virtual WaitHandle *GetProcessHandle(); // Terminate the debuggee process. virtual BOOL TerminateProcess(UINT32 exitCode); @@ -156,19 +156,48 @@ BOOL WindowsNativePipeline::ContinueDebugEvent( } // Return a handle for the debuggee process. -HANDLE WindowsNativePipeline::GetProcessHandle() +WaitHandle *WindowsNativePipeline::GetProcessHandle() { _ASSERTE(m_dwProcessId != 0); - return ::OpenProcess(PROCESS_DUP_HANDLE | - PROCESS_QUERY_INFORMATION | - PROCESS_TERMINATE | - PROCESS_VM_OPERATION | - PROCESS_VM_READ | - PROCESS_VM_WRITE | - SYNCHRONIZE, - FALSE, - m_dwProcessId); + HANDLE processHandle = ::OpenProcess(PROCESS_DUP_HANDLE | + PROCESS_QUERY_INFORMATION | + PROCESS_TERMINATE | + PROCESS_VM_OPERATION | + PROCESS_VM_READ | + PROCESS_VM_WRITE | + SYNCHRONIZE, + FALSE, + m_dwProcessId); + if (processHandle == NULL) + { + return nullptr; + } + + NativeHandle nativeHandle(processHandle); + DWORD error = GetLastError(); + CloseHandle(processHandle); + + if (!nativeHandle.IsValid()) + { + SetLastError(error); + return nullptr; + } + + WaitHandle *waitHandle = new (nothrow) WaitHandle(nativeHandle); + DWORD duplicateError = GetLastError(); + if (waitHandle == nullptr) + { + SetLastError(ERROR_NOT_ENOUGH_MEMORY); + } + else if (!waitHandle->IsValid()) + { + delete waitHandle; + waitHandle = nullptr; + SetLastError(duplicateError); + } + + return waitHandle; } // Terminate the debuggee process. diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index b4fbf49209acb5..9cbbba51090972 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -2078,8 +2078,23 @@ void HelperThreadFavor::Init() CONTRACTL_END; // Create events for managing favors. - m_FavorReadEvent = CreateWin32EventOrThrow(NULL, kAutoResetEvent, FALSE); - m_FavorAvailableEvent = CreateWin32EventOrThrow(NULL, kAutoResetEvent, FALSE); + m_FavorReadEvent = new (nothrow) WaitEvent(false); + if ((m_FavorReadEvent == nullptr) || !m_FavorReadEvent->IsValid()) + { + delete m_FavorReadEvent; + m_FavorReadEvent = nullptr; + ThrowOutOfMemory(); + } + + m_FavorAvailableEvent = new (nothrow) WaitEvent(false); + if ((m_FavorAvailableEvent == nullptr) || !m_FavorAvailableEvent->IsValid()) + { + delete m_FavorAvailableEvent; + m_FavorAvailableEvent = nullptr; + delete m_FavorReadEvent; + m_FavorReadEvent = nullptr; + ThrowOutOfMemory(); + } } @@ -6809,10 +6824,9 @@ HRESULT Debugger::LaunchJitDebuggerAndNativeAttach(Thread * pThread, EXCEPTION_P } LOG((LF_CORDB, LL_INFO10000, "D::LJDANA: waiting on m_exUnmanagedAttachEvent and debugger's process handle\n")); - DWORD dwHandles = 2; - HANDLE arrHandles[2]; - arrHandles[0] = GetUnmanagedAttachEvent(); - arrHandles[1] = processInfo.hProcess; + WaitEvent unmanagedAttachEvent(GetUnmanagedAttachEvent()); + NativeHandle debuggerProcess(processInfo.hProcess); + const WaitHandle *waitSet[] = { &unmanagedAttachEvent, &debuggerProcess }; // Let the helper thread do the attach logic for us and wait for the // attach event. Must release the lock before blocking on a wait. @@ -6820,21 +6834,24 @@ HRESULT Debugger::LaunchJitDebuggerAndNativeAttach(Thread * pThread, EXCEPTION_P // Wait for one or the other to be set. Multiple threads could be waiting here. // The events are manual events, so when they go high, all threads will be released. - DWORD res = WaitForMultipleObjectsEx(dwHandles, arrHandles, FALSE, INFINITE, FALSE); + int32_t waitResult = WaitHandle::Wait( + waitSet, + ARRAY_SIZE(waitSet), + WaitHandle::Infinite); // We no long need to keep handles to the debugger process. CloseHandle(processInfo.hProcess); CloseHandle(processInfo.hThread); // Indicate to the caller that the attach was aborted - if (res == WAIT_OBJECT_0 + 1) + if (waitResult == 1) { LOG((LF_CORDB, LL_INFO10000, "D::LJDANA: Debugger process is unexpectedly terminated!\n")); return E_FAIL; } // Otherwise, attach was successful (Note, only native attach is done so far) - _ASSERTE((res == WAIT_OBJECT_0) && "WaitForMultipleObjectsEx failed!"); + _ASSERTE((waitResult == 0) && "Debugger attach wait failed!"); LOG( (LF_CORDB, LL_INFO10000, "D::LJDANA: Leaving\n") ); return S_OK; #endif // TARGET_UNIX diff --git a/src/coreclr/debug/ee/debugger.h b/src/coreclr/debug/ee/debugger.h index d5341b4f68fee7..3adb7ff084804f 100644 --- a/src/coreclr/debug/ee/debugger.h +++ b/src/coreclr/debug/ee/debugger.h @@ -36,6 +36,7 @@ #include "eedbginterface.h" #include "dbginterface.h" #include "corhost.h" +#include "debugwait.h" #include "corjit.h" @@ -648,10 +649,10 @@ class HelperThreadFavor // that blew its stack FAVORCALLBACK m_fpFavor; void *m_pFavorData; - HANDLE m_FavorReadEvent; + WaitEvent *m_FavorReadEvent; Crst m_FavorLock; - HANDLE m_FavorAvailableEvent; + WaitEvent *m_FavorAvailableEvent; }; @@ -865,8 +866,8 @@ class DebuggerRCThread } Crst * GetFavorLock() { return &m_favorData.m_FavorLock; } - HANDLE GetFavorReadEvent() { return m_favorData.m_FavorReadEvent; } - HANDLE GetFavorAvailableEvent() { return m_favorData.m_FavorAvailableEvent; } + WaitEvent *GetFavorReadEvent() { return m_favorData.m_FavorReadEvent; } + WaitEvent *GetFavorAvailableEvent() { return m_favorData.m_FavorAvailableEvent; } HelperThreadFavor m_favorData; @@ -891,9 +892,11 @@ class DebuggerRCThread #endif // FEATURE_DBGIPC_TRANSPORT_VM HANDLE m_thread; + Volatile m_helperThreadRunning; bool m_run; - HANDLE m_threadControlEvent; + WaitEvent *m_threadControlEvent; + WaitEvent *m_helperThreadExitedEvent; HANDLE m_helperThreadCanGoEvent; bool m_rgfInitRuntimeOffsets[IPC_TARGET_COUNT]; bool m_fDetachRightSide; diff --git a/src/coreclr/debug/ee/rcthread.cpp b/src/coreclr/debug/ee/rcthread.cpp index e60996b61960db..26fcdcb1628aea 100644 --- a/src/coreclr/debug/ee/rcthread.cpp +++ b/src/coreclr/debug/ee/rcthread.cpp @@ -30,8 +30,10 @@ DebuggerRCThread::DebuggerRCThread(Debugger * pDebugger) : m_debugger(pDebugger), m_pDCB(NULL), m_thread(NULL), + m_helperThreadRunning(FALSE), m_run(true), m_threadControlEvent(NULL), + m_helperThreadExitedEvent(NULL), m_helperThreadCanGoEvent(NULL), m_fDetachRightSide(false) { @@ -227,7 +229,7 @@ void DebuggerRCThread::WatchForStragglers(void) LOG((LF_CORDB,LL_INFO100000, "DRCT::WFS:setting event to watch " "for stragglers\n")); - SetEvent(m_threadControlEvent); + m_threadControlEvent->Set(); } //--------------------------------------------------------------------------------------- @@ -274,7 +276,24 @@ HRESULT DebuggerRCThread::Init(void) // Create the thread control event. - m_threadControlEvent = CreateWin32EventOrThrow(NULL, kAutoResetEvent, FALSE); + m_threadControlEvent = new (nothrow) WaitEvent(false); + if ((m_threadControlEvent == nullptr) || !m_threadControlEvent->IsValid()) + { + delete m_threadControlEvent; + m_threadControlEvent = nullptr; + ThrowOutOfMemory(); + } + + // Track liveness separately so this auto-reset event is only an exit notification. + m_helperThreadExitedEvent = new (nothrow) WaitEvent(false); + if ((m_helperThreadExitedEvent == nullptr) || !m_helperThreadExitedEvent->IsValid()) + { + delete m_helperThreadExitedEvent; + m_helperThreadExitedEvent = nullptr; + delete m_threadControlEvent; + m_threadControlEvent = nullptr; + ThrowOutOfMemory(); + } // Create the helper thread can go event. m_helperThreadCanGoEvent = CreateWin32EventOrThrow(NULL, kManualResetEvent, TRUE); @@ -564,6 +583,18 @@ void DebuggerRCThread::ThreadProc(void) } CONTRACTL_END; + struct HelperThreadExitSignal + { + Volatile &Running; + WaitEvent *Event; + + ~HelperThreadExitSignal() + { + Running.Store(FALSE); + Event->Set(); + } + }; + STRESS_LOG_RESERVE_MEM (0); // This message actually serves a purpose (which is why it is always run) // The Stress log is run during hijacking, when other threads can be suspended @@ -651,7 +682,9 @@ void DebuggerRCThread::ThreadProc(void) // Mark that we're the true helper thread. Now that we've marked // this, no other threads will ever become the temporary helper // thread. + HelperThreadExitSignal helperThreadExitSignal = { m_helperThreadRunning, m_helperThreadExitedEvent }; m_pDCB->m_helperThreadId = GetCurrentThreadId(); + m_helperThreadRunning.Store(TRUE); LOG((LF_CORDB, LL_INFO1000, "DRCT::TP: helper thread id is 0x%x helperThreadId\n", m_pDCB->m_helperThreadId)); @@ -827,7 +860,13 @@ void DebuggerRCThread::MainLoop() // threads doing helper duty. CantStopHolder cantStopHolder; - HANDLE rghWaitSet[DRCT_COUNT_FINAL]; + const WaitHandle *waitSet[DRCT_COUNT_FINAL]; +#if !defined(FEATURE_DBGIPC_TRANSPORT_VM) + WaitEvent rightSideEventAvailable(m_pDCB->m_rightSideEventAvailable.ImportToLocalProcess()); +#ifdef HOST_WINDOWS + NativeHandle *debuggerProcess = nullptr; +#endif // HOST_WINDOWS +#endif #ifdef _DEBUG DWORD dwSyncSpinCount = 0; @@ -836,12 +875,12 @@ void DebuggerRCThread::MainLoop() // We start out just listening on RSEA and the thread control event... unsigned int cWaitCount = DRCT_COUNT_INITIAL; DWORD dwWaitTimeout = INFINITE; - rghWaitSet[DRCT_CONTROL_EVENT] = m_threadControlEvent; - rghWaitSet[DRCT_FAVORAVAIL] = GetFavorAvailableEvent(); + waitSet[DRCT_CONTROL_EVENT] = m_threadControlEvent; + waitSet[DRCT_FAVORAVAIL] = GetFavorAvailableEvent(); #if !defined(FEATURE_DBGIPC_TRANSPORT_VM) - rghWaitSet[DRCT_RSEA] = m_pDCB->m_rightSideEventAvailable; + waitSet[DRCT_RSEA] = &rightSideEventAvailable; #else - rghWaitSet[DRCT_RSEA] = g_pDbgTransport->GetIPCEventReadyEvent(); + waitSet[DRCT_RSEA] = g_pDbgTransport->GetIPCEventReadyEvent(); #endif // !FEATURE_DBGIPC_TRANSPORT_VM CONTRACT_VIOLATION(ThrowsViolation);// HndCreateHandle throws, and this loop is not backstopped by any EH @@ -854,33 +893,42 @@ void DebuggerRCThread::MainLoop() { LOG((LF_CORDB, LL_INFO1000, "DRCT::ML: waiting for event.\n")); -#if !defined(FEATURE_DBGIPC_TRANSPORT_VM) +#if !defined(FEATURE_DBGIPC_TRANSPORT_VM) && defined(HOST_WINDOWS) // If there is a debugger attached, wait on its handle, too... if ((cWaitCount == DRCT_COUNT_INITIAL) && m_pDCB->m_rightSideProcessHandle.ImportToLocalProcess() != NULL) { _ASSERTE((cWaitCount + 1) == DRCT_COUNT_FINAL); - rghWaitSet[DRCT_DEBUGGER_EVENT] = m_pDCB->m_rightSideProcessHandle; + debuggerProcess = new (nothrow) NativeHandle( + m_pDCB->m_rightSideProcessHandle.ImportToLocalProcess()); + if ((debuggerProcess == nullptr) || !debuggerProcess->IsValid()) + { + delete debuggerProcess; + EEPOLICY_HANDLE_FATAL_ERROR(COR_E_OUTOFMEMORY); + } + waitSet[DRCT_DEBUGGER_EVENT] = debuggerProcess; cWaitCount = DRCT_COUNT_FINAL; } -#endif // !FEATURE_DBGIPC_TRANSPORT_VM +#endif // !defined(FEATURE_DBGIPC_TRANSPORT_VM) && defined(HOST_WINDOWS) if (m_fDetachRightSide) { m_fDetachRightSide = false; -#if !defined(FEATURE_DBGIPC_TRANSPORT_VM) +#if !defined(FEATURE_DBGIPC_TRANSPORT_VM) && defined(HOST_WINDOWS) _ASSERTE(cWaitCount == DRCT_COUNT_FINAL); _ASSERTE((cWaitCount - 1) == DRCT_COUNT_INITIAL); - rghWaitSet[DRCT_DEBUGGER_EVENT] = NULL; + delete debuggerProcess; + debuggerProcess = nullptr; + waitSet[DRCT_DEBUGGER_EVENT] = nullptr; cWaitCount = DRCT_COUNT_INITIAL; -#endif // !FEATURE_DBGIPC_TRANSPORT_VM +#endif // !defined(FEATURE_DBGIPC_TRANSPORT_VM) && defined(HOST_WINDOWS) } // Wait for an event from the Right Side. - DWORD dwWaitResult = WaitForMultipleObjectsEx(cWaitCount, rghWaitSet, FALSE, dwWaitTimeout, FALSE); + int32_t waitResult = WaitHandle::Wait(waitSet, cWaitCount, dwWaitTimeout); if (!m_run) { @@ -888,7 +936,7 @@ void DebuggerRCThread::MainLoop() } - if (dwWaitResult == WAIT_OBJECT_0 + DRCT_DEBUGGER_EVENT) + if (waitResult == DRCT_DEBUGGER_EVENT) { // If the handle of the right side process is signaled, then we've lost our controlling debugger. We // terminate this process immediately in such a case. @@ -897,7 +945,7 @@ void DebuggerRCThread::MainLoop() EEPOLICY_HANDLE_FATAL_ERROR(0); _ASSERTE(!"Should never reach this point."); } - else if (dwWaitResult == WAIT_OBJECT_0 + DRCT_FAVORAVAIL) + else if (waitResult == DRCT_FAVORAVAIL) { // execute the callback set by DoFavor() FAVORCALLBACK fpCallback = GetFavorFnPtr(); @@ -907,10 +955,10 @@ void DebuggerRCThread::MainLoop() if (fpCallback) { (*fpCallback)(GetFavorData()); - SetEvent(GetFavorReadEvent()); + GetFavorReadEvent()->Set(); } } - else if (dwWaitResult == WAIT_OBJECT_0 + DRCT_RSEA) + else if (waitResult == DRCT_RSEA) { bool fWasContinue = HandleRSEA(); @@ -940,7 +988,7 @@ void DebuggerRCThread::MainLoop() } } - else if (dwWaitResult == WAIT_OBJECT_0 + DRCT_CONTROL_EVENT) + else if (waitResult == DRCT_CONTROL_EVENT) { LOG((LF_CORDB, LL_INFO1000, "DRCT::ML:: straggler event set.\n")); @@ -966,7 +1014,7 @@ void DebuggerRCThread::MainLoop() // dbgLockHolder goes out of scope - implicit Release // tsl goes out of scope - implicit Release } - else if (dwWaitResult == WAIT_TIMEOUT) + else if (waitResult == WaitHandle::Timeout) { LWaitTimedOut: @@ -1036,6 +1084,10 @@ void DebuggerRCThread::MainLoop() } } +#if !defined(FEATURE_DBGIPC_TRANSPORT_VM) && defined(HOST_WINDOWS) + delete debuggerProcess; +#endif // !defined(FEATURE_DBGIPC_TRANSPORT_VM) && defined(HOST_WINDOWS) + STRESS_LOG0(LF_CORDB, LL_INFO1000, "DRCT::ML:: Exiting.\n"); } @@ -1076,7 +1128,10 @@ void DebuggerRCThread::TemporaryHelperThreadMainLoop() // threads doing helper duty. CantStopHolder cantStopHolder; - HANDLE rghWaitSet[DRCT_COUNT_FINAL]; + const WaitHandle *waitSet[DRCT_COUNT_FINAL]; +#if !defined(FEATURE_DBGIPC_TRANSPORT_VM) + WaitEvent rightSideEventAvailable(m_pDCB->m_rightSideEventAvailable.ImportToLocalProcess()); +#endif #ifdef _DEBUG DWORD dwSyncSpinCount = 0; @@ -1085,12 +1140,12 @@ void DebuggerRCThread::TemporaryHelperThreadMainLoop() // We start out just listening on RSEA and the thread control event... unsigned int cWaitCount = DRCT_COUNT_INITIAL; DWORD dwWaitTimeout = INFINITE; - rghWaitSet[DRCT_CONTROL_EVENT] = m_threadControlEvent; - rghWaitSet[DRCT_FAVORAVAIL] = GetFavorAvailableEvent(); + waitSet[DRCT_CONTROL_EVENT] = m_threadControlEvent; + waitSet[DRCT_FAVORAVAIL] = GetFavorAvailableEvent(); #if !defined(FEATURE_DBGIPC_TRANSPORT_VM) - rghWaitSet[DRCT_RSEA] = m_pDCB->m_rightSideEventAvailable; + waitSet[DRCT_RSEA] = &rightSideEventAvailable; #else //FEATURE_DBGIPC_TRANSPORT_VM - rghWaitSet[DRCT_RSEA] = g_pDbgTransport->GetIPCEventReadyEvent(); + waitSet[DRCT_RSEA] = g_pDbgTransport->GetIPCEventReadyEvent(); #endif // !FEATURE_DBGIPC_TRANSPORT_VM CONTRACT_VIOLATION(ThrowsViolation);// HndCreateHandle throws, and this loop is not backstopped by any EH @@ -1100,7 +1155,7 @@ void DebuggerRCThread::TemporaryHelperThreadMainLoop() LOG((LF_CORDB, LL_INFO1000, "DRCT::ML: waiting for event.\n")); // Wait for an event from the Right Side. - DWORD dwWaitResult = WaitForMultipleObjectsEx(cWaitCount, rghWaitSet, FALSE, dwWaitTimeout, FALSE); + int32_t waitResult = WaitHandle::Wait(waitSet, cWaitCount, dwWaitTimeout); if (!m_run) { @@ -1108,7 +1163,7 @@ void DebuggerRCThread::TemporaryHelperThreadMainLoop() } - if (dwWaitResult == WAIT_OBJECT_0 + DRCT_DEBUGGER_EVENT) + if (waitResult == DRCT_DEBUGGER_EVENT) { // If the handle of the right side process is signaled, then we've lost our controlling debugger. We // terminate this process immediately in such a case. @@ -1117,14 +1172,14 @@ void DebuggerRCThread::TemporaryHelperThreadMainLoop() TerminateProcess(GetCurrentProcess(), 0); _ASSERTE(!"Should never reach this point."); } - else if (dwWaitResult == WAIT_OBJECT_0 + DRCT_FAVORAVAIL) + else if (waitResult == DRCT_FAVORAVAIL) { // execute the callback set by DoFavor() (*GetFavorFnPtr())(GetFavorData()); - SetEvent(GetFavorReadEvent()); + GetFavorReadEvent()->Set(); } - else if (dwWaitResult == WAIT_OBJECT_0 + DRCT_RSEA) + else if (waitResult == DRCT_RSEA) { // @todo: // We are only interested in dealing with Continue event here... @@ -1147,7 +1202,7 @@ void DebuggerRCThread::TemporaryHelperThreadMainLoop() goto LExit; } } - else if (dwWaitResult == WAIT_OBJECT_0 + DRCT_CONTROL_EVENT) + else if (waitResult == DRCT_CONTROL_EVENT) { LOG((LF_CORDB, LL_INFO1000, "DRCT::THTML:: straggler event set.\n")); @@ -1163,7 +1218,7 @@ void DebuggerRCThread::TemporaryHelperThreadMainLoop() // goto LWaitTimedOut; } - else if (dwWaitResult == WAIT_TIMEOUT) + else if (waitResult == WaitHandle::Timeout) { LWaitTimedOut: @@ -1385,7 +1440,7 @@ HRESULT DebuggerRCThread::AsyncStop(void) // We need to get the helper thread out of its wait loop. So ping the thread-control event. // (Don't ping RSEA since that event should be used only for IPC communication). // Don't bother waiting for it to exit. - SetEvent(this->m_threadControlEvent); + this->m_threadControlEvent->Set(); return hr; } @@ -1544,6 +1599,11 @@ bool DebuggerRCThread::IsRCThreadReady() return false; } + if (!m_helperThreadRunning.Load()) + { + return false; + } + // a more subtle check. It's possible the thread was up, but then // an bad call to ExitProcess suddenly terminated the helper thread, // leaving the threadid still non-0. So check the actual thread object @@ -1623,88 +1683,105 @@ void DebuggerRCThread::DoFavor(FAVORCALLBACK fp, void * pData) // We are being called on managed thread only. // - // We'll have problems if another thread comes in and - // deletes the RCThread object on us while we're in this call. - if (IsRCThreadReady()) - { - // If the helper thread calls this, we deadlock. - // (Since we wait on an event that only the helper thread sets) - _ASSERTE(GetRCThreadId() != GetCurrentThreadId()); + bool executeFavorOnCurrentThread = !IsRCThreadReady(); - // Only lock if we're waiting on the helper thread. + if (!executeFavorOnCurrentThread) + { + // Serialize the readiness check with publishing and waiting for a favor. // This should be the only place the FavorLock is used. // Note this is never called on the helper thread. CrstHolder ch(GetFavorLock()); - SetFavorFnPtr(fp, pData); + // Check readiness while holding the favor lock so that a prior waiter cannot consume + // the auto-reset exit notification before this caller starts waiting. + if (IsRCThreadReady()) + { + // If the helper thread calls this, we deadlock. + // (Since we wait on an event that only the helper thread sets) + _ASSERTE(GetRCThreadId() != GetCurrentThreadId()); + + SetFavorFnPtr(fp, pData); - // Our main message loop operating on the Helper thread will - // pickup that event, call the fp, and set the Read event - SetEvent(GetFavorAvailableEvent()); + // Our main message loop operating on the Helper thread will + // pickup that event, call the fp, and set the Read event + GetFavorAvailableEvent()->Set(); - LOG((LF_CORDB, LL_INFO10000, "DRCT::DF - Waiting on FavorReadEvent for favor %p\n", (void*)fp)); + LOG((LF_CORDB, LL_INFO10000, "DRCT::DF - Waiting on FavorReadEvent for favor %p\n", (void*)fp)); - // Wait for either the FavorEventRead to be set (which means that the favor - // was executed by the helper thread) or the helper thread's handle (which means - // that the helper thread exited without doing the favor, so we should do it) - // - // Note we are assuming that there's only 2 ways the helper thread can exit: - // 1) Someone calls ::ExitProcess, killing all threads. That will kill us too, so we're "ok". - // 2) Someone calls Stop(), causing the helper to exit gracefully. That's ok too. The helper - // didn't execute the Favor (else the FREvent would have been set first) and so we can. - // - // Beware of problems: - // 1) If the helper can block, we may deadlock. - // 2) If the helper can exit magically (or if we change the Wait to include a timeout) , - // the helper thread may have not executed the favor, partially executed the favor, - // or totally executed the favor but not yet signaled the FavorReadEvent. We don't - // know what it did, so we don't know what we can do; so we're in an unstable state. - - const HANDLE waitset [] = { GetFavorReadEvent(), m_thread }; - - // the favor worker thread will require a transition to cooperative mode in order to complete its work and we will - // wait for the favor to complete before terminating the process. if there is a GC in progress the favor thread - // will be blocked and if the thread requesting the favor is in cooperative mode we'll deadlock, so we switch to - // preemptive mode before waiting for the favor to complete (see Dev11 72349). - GCX_PREEMP(); - - DWORD ret = WaitForMultipleObjectsEx( - ARRAY_SIZE(waitset), - waitset, - FALSE, - INFINITE, - FALSE - ); - - DWORD wn = (ret - WAIT_OBJECT_0); - if (wn == 0) // m_FavorEventRead - { - // Favor was executed, nothing to do here. - LOG((LF_CORDB, LL_INFO10000, "DRCT::DF - favor %p finished, ret = %d\n", (void*)fp, ret)); - } - else - { - LOG((LF_CORDB, LL_INFO10000, "DRCT::DF - lost helper thread during wait, " - "doing favor %p on current thread\n", (void*)fp)); + // Wait for either the FavorEventRead to be set (which means that the favor + // was executed by the helper thread) or the helper thread's handle (which means + // that the helper thread exited without doing the favor, so we should do it) + // + // Note we are assuming that there's only 2 ways the helper thread can exit: + // 1) Someone calls ::ExitProcess, killing all threads. That will kill us too, so we're "ok". + // 2) Someone calls Stop(), causing the helper to exit gracefully. That's ok too. The helper + // didn't execute the Favor (else the FREvent would have been set first) and so we can. + // + // Beware of problems: + // 1) If the helper can block, we may deadlock. + // 2) If the helper can exit magically (or if we change the Wait to include a timeout) , + // the helper thread may have not executed the favor, partially executed the favor, + // or totally executed the favor but not yet signaled the FavorReadEvent. We don't + // know what it did, so we don't know what we can do; so we're in an unstable state. + + const WaitHandle *waitSet[] = { + GetFavorReadEvent(), +#ifdef HOST_WINDOWS + // Preserve detection of abnormal helper termination through the native thread handle. + nullptr +#else + m_helperThreadExitedEvent +#endif + }; +#ifdef HOST_WINDOWS + NativeHandle helperThread(m_thread); + waitSet[1] = &helperThread; +#endif - // Since we have no timeout, we shouldn't be able to get an error on the wait, - // but just in case ... - _ASSERTE(ret != WAIT_FAILED); - _ASSERTE((wn == 1) && !"DoFavor - unexpected return from WFMO"); + // the favor worker thread will require a transition to cooperative mode in order to complete its work and we will + // wait for the favor to complete before terminating the process. if there is a GC in progress the favor thread + // will be blocked and if the thread requesting the favor is in cooperative mode we'll deadlock, so we switch to + // preemptive mode before waiting for the favor to complete (see Dev11 72349). + GCX_PREEMP(); - // Thread exited without doing favor, so execute it on our thread. - // If we're here because of a stack overflow, this may push us over the edge, - // but there's nothing else we can really do - (*fp)(pData); + int32_t waitResult = WaitHandle::Wait( + waitSet, + ARRAY_SIZE(waitSet), + WaitHandle::Infinite); - ResetEvent(GetFavorAvailableEvent()); - } + if (waitResult == 0) + { + // Favor was executed, nothing to do here. + LOG((LF_CORDB, LL_INFO10000, "DRCT::DF - favor %p finished, ret = %d\n", (void*)fp, waitResult)); + } + else + { + LOG((LF_CORDB, LL_INFO10000, "DRCT::DF - lost helper thread during wait, " + "doing favor %p on current thread\n", (void*)fp)); + + // Since we have no timeout, we shouldn't be able to get an error on the wait, + // but just in case ... + _ASSERTE(waitResult != WaitHandle::Failed); + _ASSERTE((waitResult == 1) || !"DoFavor - unexpected wait result"); - // m_fpFavor & m_pFavorData are meaningless now. We could set them - // to NULL, but we may as well leave them as is to leave a trail. + // Thread exited without doing favor, so execute it on our thread. + // If we're here because of a stack overflow, this may push us over the edge, + // but there's nothing else we can really do + (*fp)(pData); + GetFavorAvailableEvent()->Reset(); + } + + // m_fpFavor & m_pFavorData are meaningless now. We could set them + // to NULL, but we may as well leave them as is to leave a trail. + } + else + { + executeFavorOnCurrentThread = true; + } } - else + + if (executeFavorOnCurrentThread) { LOG((LF_CORDB, LL_INFO10000, "DRCT::DF - helper thread not ready, " "doing favor %p on current thread\n", (void*)fp)); @@ -1772,6 +1849,8 @@ void DebuggerRCThread::EarlyHelperThreadDeath(void) { LOG((LF_CORDB, LL_INFO10000, "DRCT::EHTD\n")); + m_helperThreadRunning.Store(FALSE); + // If we ever spun up a thread... if (m_thread != NULL && m_pDCB) { @@ -1783,4 +1862,3 @@ void DebuggerRCThread::EarlyHelperThreadDeath(void) // dbgLockHolder goes out of scope - implicit Release } } - diff --git a/src/coreclr/debug/inc/dbgtransportsession.h b/src/coreclr/debug/inc/dbgtransportsession.h index 74bb8ab2f7b09b..d349fb47cc61c5 100644 --- a/src/coreclr/debug/inc/dbgtransportsession.h +++ b/src/coreclr/debug/inc/dbgtransportsession.h @@ -11,7 +11,9 @@ #endif // !RIGHT_SIDE_COMPILE #include +#include #include +#include "debugwait.h" #if defined(FEATURE_DBGIPC_TRANSPORT_VM) || defined(FEATURE_DBGIPC_TRANSPORT_DI) @@ -279,6 +281,13 @@ class DbgTransportLock final void Enter(); void Leave(); +#ifdef RIGHT_SIDE_COMPILE + minipal_mutex& GetMutex() + { + return m_sLock; + } +#endif // RIGHT_SIDE_COMPILE + private: #ifdef RIGHT_SIDE_COMPILE minipal_mutex m_sLock; @@ -358,7 +367,7 @@ class DbgTransportSession // requires the addresses of a couple of runtime data structures to service certain debugger requests that // may be delivered once the session is established. #ifdef RIGHT_SIDE_COMPILE - HRESULT Init(const ProcessDescriptor& pd, HANDLE hProcessExited); + HRESULT Init(const ProcessDescriptor& pd, const WaitHandle& processExited); #else HRESULT Init(DebuggerIPCControlBlock * pDCB); #endif // RIGHT_SIDE_COMPILE @@ -424,8 +433,8 @@ class DbgTransportSession // Retrieves the auto-reset handle which is signalled by the session each time a new event is received // from the other side. - HANDLE GetIPCEventReadyEvent(); - HANDLE GetDebugEventReadyEvent(); + WaitEvent *GetIPCEventReadyEvent(); + WaitEvent *GetDebugEventReadyEvent(); // Copies the last event received from the other side into the provided buffer. This should only be called // (once) after the event returned from GetIPCEventReadyEvent()/GetDebugEventReadyEvent() has been signalled. @@ -560,7 +569,7 @@ class DbgTransportSession MessageHeader m_sHeader; // Inline message header PBYTE m_pbDataBlock; // Pointer to optional message data block (or NULL) DWORD m_cbDataBlock; // Count of bytes in above block if it's non-NULL - HANDLE m_hReplyEvent; // Optional event to signal if this message is replied to (or NULL) + WaitEvent *m_hReplyEvent; // Optional event to signal if this message is replied to (or NULL) PBYTE m_pbReplyBlock; // Optional buffer to place data block from reply into (or NULL) DWORD m_cbReplyBlock; // Size in bytes of the above buffer if it is non-NULL Message *m_pOrigMessage; // Used when we need to find the original message from a copy @@ -587,7 +596,7 @@ class DbgTransportSession LONG m_ref; // Some flags used to record how far we got in Init() (used for cleanup in Shutdown()). - bool m_fInitStateLock; + bool m_fInitStateLock = false; // Protocol version. This consists of two parts. The major version is incremented on incompatible protocol // updates. That is, a session between left and right sides that cannot use a protocol with the exact same @@ -640,23 +649,23 @@ class DbgTransportSession SessionState m_eState; #ifdef RIGHT_SIDE_COMPILE - // Manual reset event that is signalled whenever the session state is SS_Open or SS_Closed (after waiting - // on this event the caller should check to see which state it was). - HANDLE m_hSessionOpenEvent; + // Notified whenever the session reaches a state that resolves WaitForSessionToOpen(). + minipal_condition_variable m_sessionStateCondition; + bool m_fInitSessionStateCondition = false; #endif // RIGHT_SIDE_COMPILE // Thread responsible for initial Connect()/Accept() on a low level transport connection and // subsequently for all message reception on that connection. Any error will cause the thread to reset // back into the Connect()/Accept() phase (along with the resulting session state change). - HANDLE m_hTransportThread; + HANDLE m_hTransportThread = NULL; - IDebugChannel* m_channel; + IDebugChannel* m_channel = NULL; #ifdef RIGHT_SIDE_COMPILE // On the RS the transport thread needs to know the IP address and port number to Connect() to. ProcessDescriptor m_pd; // Descriptor of a process we're talking to. - HANDLE m_hProcessExited; // event which will be signaled when the debuggee is terminated + WaitHandle *m_hProcessExited = NULL; // wait which will be signaled when the debuggee is terminated bool m_fDebuggerAttached; #endif @@ -676,12 +685,12 @@ class DbgTransportSession // at any one time). The buffer is a circular array: clients read from the buffer at head index which is // followed by some number of valid buffers (wrapping around to the start of the array if necessary). New // events are added after these (and grow the array if the tail would touch the head otherwise). - DbgEventBufferEntry * m_pEventBuffers; // Pointer to array of incoming debugger events + DbgEventBufferEntry * m_pEventBuffers = NULL; // Pointer to array of incoming debugger events DWORD m_cEventBuffers; // Size of the array above (in events) DWORD m_cValidEventBuffers; // Number of events that actually contain data DWORD m_idxEventBufferHead; // Index of the first valid event DWORD m_idxEventBufferTail; // Index of the first invalid event - HANDLE m_rghEventReadyEvent[IPCET_Max]; // The event signalled when a new event arrives + WaitEvent *m_rghEventReadyEvent[IPCET_Max] = {}; // The event signalled when a new event arrives #ifndef RIGHT_SIDE_COMPILE // The LS requires the addresses of a couple of runtime data structures in order to service MT_GetDCB etc. @@ -811,6 +820,8 @@ class DbgTransportSession // Initialize all session state to correct starting values. Used during Init() and on the LS when we // gracefully close one session and prepare for another. void InitSessionState(); + void SetSessionState(SessionState state); + void SetSessionStateUnderLock(SessionState state); // The entry point of the transport worker thread. This one's static, so we immediately dispatch to an // instance method version defined below for convenience in the implementation. diff --git a/src/coreclr/debug/inc/debugwait.h b/src/coreclr/debug/inc/debugwait.h new file mode 100644 index 00000000000000..0e9d51f409fe71 --- /dev/null +++ b/src/coreclr/debug/inc/debugwait.h @@ -0,0 +1,113 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#ifndef _DEBUGWAIT_H_ +#define _DEBUGWAIT_H_ + +#include + +#ifdef HOST_WINDOWS +#include +#endif + +class WaitHandle +{ +public: + static constexpr uint32_t Infinite = UINT32_MAX; + static constexpr int32_t Timeout = -1; + static constexpr int32_t Failed = -2; + + bool IsValid() const + { + return m_handle != nullptr; + } + +#ifdef HOST_WINDOWS + HANDLE GetRawHandle() const + { + return m_handle; + } +#endif + + // Waits return the zero-based index of the acquired handle, or one of the negative results above. + // Handles must remain valid until the wait returns. + static int32_t Wait(const WaitHandle& handle, uint32_t timeout) + { + const WaitHandle* handles[] = { &handle }; + return Wait(handles, 1, timeout); + } + + static int32_t Wait( + const WaitHandle* const* handles, + uint32_t count, + uint32_t timeout); + + WaitHandle(const WaitHandle& handle); + WaitHandle& operator=(const WaitHandle& handle) = delete; + virtual ~WaitHandle(); + +protected: +#ifdef HOST_WINDOWS + explicit WaitHandle(HANDLE handle); +#else + explicit WaitHandle(void* handle); +#endif + +#ifndef HOST_WINDOWS + void* GetWaitable() const + { + return m_handle; + } +#endif + +private: +#ifdef HOST_WINDOWS + HANDLE m_handle; +#else + void* m_handle; +#endif +}; + +class WaitEvent final : public WaitHandle +{ +public: + explicit WaitEvent(bool initialState); + +#ifdef HOST_WINDOWS + // Duplicates an existing native event handle. + explicit WaitEvent(HANDLE handle); +#else + // A non-Windows waitable is a debug PAL primitive, not a PAL HANDLE, so an existing + // handle can't be imported. Reject pointers rather than letting them silently select + // the initial state constructor and create an unrelated event. + explicit WaitEvent(void* handle) = delete; +#endif + WaitEvent(const WaitEvent& event) = default; + WaitEvent& operator=(const WaitEvent& event) = delete; + + bool Set(); + bool Reset(); +}; + +class WaitLatch final : public WaitHandle +{ +public: + WaitLatch(); + WaitLatch(const WaitLatch& latch) = default; + WaitLatch& operator=(const WaitLatch& latch) = delete; + + bool Set(); +}; + +#ifdef HOST_WINDOWS +class NativeHandle final : public WaitHandle +{ +public: + // Duplicates an existing native handle. + explicit NativeHandle(HANDLE handle); + NativeHandle(const NativeHandle& handle) = default; + NativeHandle& operator=(const NativeHandle& handle) = delete; +}; +#endif // HOST_WINDOWS + +#endif // _DEBUGWAIT_H_ diff --git a/src/coreclr/debug/shared/dbgtransportsession.cpp b/src/coreclr/debug/shared/dbgtransportsession.cpp index 6f8083f0690f92..c2a3578d4adee9 100644 --- a/src/coreclr/debug/shared/dbgtransportsession.cpp +++ b/src/coreclr/debug/shared/dbgtransportsession.cpp @@ -4,6 +4,10 @@ #include "dbgtransportsession.h" +#ifdef RIGHT_SIDE_COMPILE +#include +#endif // RIGHT_SIDE_COMPILE + #if (!defined(RIGHT_SIDE_COMPILE) && defined(FEATURE_DBGIPC_TRANSPORT_VM)) || (defined(RIGHT_SIDE_COMPILE) && defined(FEATURE_DBGIPC_TRANSPORT_DI)) // This is the entry type for the IPC event queue owned by the transport. @@ -53,18 +57,18 @@ DbgTransportSession::~DbgTransportSession() if (m_hTransportThread) CloseHandle(m_hTransportThread); if (m_rghEventReadyEvent[IPCET_OldStyle]) - CloseHandle(m_rghEventReadyEvent[IPCET_OldStyle]); + delete m_rghEventReadyEvent[IPCET_OldStyle]; if (m_rghEventReadyEvent[IPCET_DebugEvent]) - CloseHandle(m_rghEventReadyEvent[IPCET_DebugEvent]); + delete m_rghEventReadyEvent[IPCET_DebugEvent]; if (m_pEventBuffers) delete [] m_pEventBuffers; #ifdef RIGHT_SIDE_COMPILE - if (m_hSessionOpenEvent) - CloseHandle(m_hSessionOpenEvent); + if (m_fInitSessionStateCondition) + minipal_condition_variable_destroy(&m_sessionStateCondition); if (m_hProcessExited) - CloseHandle(m_hProcessExited); + delete m_hProcessExited; #endif // RIGHT_SIDE_COMPILE if (m_fInitStateLock) @@ -81,7 +85,7 @@ DbgTransportSession::~DbgTransportSession() // addresses of a couple of runtime data structures to service certain debugger requests that may be delivered // once the session is established. #ifdef RIGHT_SIDE_COMPILE -HRESULT DbgTransportSession::Init(const ProcessDescriptor& pd, HANDLE hProcessExited) +HRESULT DbgTransportSession::Init(const ProcessDescriptor& pd, const WaitHandle& processExited) #else // RIGHT_SIDE_COMPILE HRESULT DbgTransportSession::Init(DebuggerIPCControlBlock *pDCB) #endif // RIGHT_SIDE_COMPILE @@ -97,6 +101,10 @@ HRESULT DbgTransportSession::Init(DebuggerIPCControlBlock *pDCB) m_fInitStateLock = true; #ifdef RIGHT_SIDE_COMPILE + if (!minipal_condition_variable_init(&m_sessionStateCondition)) + return E_OUTOFMEMORY; + m_fInitSessionStateCondition = true; + // The RS randomly allocates a session ID which is sent to the LS in the SessionRequest message. In the // case of network errors during session formation this allows the LS to tell SessionRequest re-sends from // a new request from a different RS. @@ -104,22 +112,15 @@ HRESULT DbgTransportSession::Init(DebuggerIPCControlBlock *pDCB) return E_FAIL; m_pd = pd; - if (!DuplicateHandle(GetCurrentProcess(), - hProcessExited, - GetCurrentProcess(), - &m_hProcessExited, - 0, // ignored since we are going to pass DUPLICATE_SAME_ACCESS - FALSE, - DUPLICATE_SAME_ACCESS)) + m_hProcessExited = new (nothrow) WaitHandle(processExited); + if ((m_hProcessExited == nullptr) || !m_hProcessExited->IsValid()) { - return HRESULT_FROM_GetLastError(); + delete m_hProcessExited; + m_hProcessExited = nullptr; + return E_FAIL; } m_fDebuggerAttached = false; - m_hSessionOpenEvent = CreateEvent(NULL, TRUE, FALSE, NULL); // Manual reset, not signalled - if (m_hSessionOpenEvent == NULL) - return E_OUTOFMEMORY; - #else // RIGHT_SIDE_COMPILE m_pDCB = pDCB; @@ -139,13 +140,23 @@ HRESULT DbgTransportSession::Init(DebuggerIPCControlBlock *pDCB) if (m_pEventBuffers == NULL) return E_OUTOFMEMORY; - m_rghEventReadyEvent[IPCET_OldStyle] = CreateEvent(NULL, FALSE, FALSE, NULL); // Auto reset, not signalled - if (m_rghEventReadyEvent[IPCET_OldStyle] == NULL) + m_rghEventReadyEvent[IPCET_OldStyle] = new (nothrow) WaitEvent(false); + if ((m_rghEventReadyEvent[IPCET_OldStyle] == nullptr) || + !m_rghEventReadyEvent[IPCET_OldStyle]->IsValid()) + { + delete m_rghEventReadyEvent[IPCET_OldStyle]; + m_rghEventReadyEvent[IPCET_OldStyle] = nullptr; return E_OUTOFMEMORY; + } - m_rghEventReadyEvent[IPCET_DebugEvent] = CreateEvent(NULL, FALSE, FALSE, NULL); // Auto reset, not signalled - if (m_rghEventReadyEvent[IPCET_DebugEvent] == NULL) + m_rghEventReadyEvent[IPCET_DebugEvent] = new (nothrow) WaitEvent(false); + if ((m_rghEventReadyEvent[IPCET_DebugEvent] == nullptr) || + !m_rghEventReadyEvent[IPCET_DebugEvent]->IsValid()) + { + delete m_rghEventReadyEvent[IPCET_DebugEvent]; + m_rghEventReadyEvent[IPCET_DebugEvent] = nullptr; return E_OUTOFMEMORY; + } // Start the transport thread which handles forming and re-forming connections, driving the session // state to SS_Open and receiving and initially processing all incoming traffic. @@ -192,7 +203,7 @@ void DbgTransportSession::Shutdown() // Remember previous state and transition to SS_Closed. SessionState ePreviousState = m_eState; - m_eState = SS_Closed; + SetSessionStateUnderLock(SS_Closed); if (ePreviousState != SS_Closed && m_channel != NULL) { @@ -200,11 +211,6 @@ void DbgTransportSession::Shutdown() } } // Leave m_sStateLock - -#ifdef RIGHT_SIDE_COMPILE - // Signal the m_hSessionOpenEvent now to quickly error out any callers of WaitForSessionToOpen(). - SetEvent(m_hSessionOpenEvent); -#endif // RIGHT_SIDE_COMPILE } // The transport instance is no longer valid @@ -221,7 +227,7 @@ void DbgTransportSession::Neuter() // Simply set the session state to SS_Closed. The transport thread will switch itself off if it ever gets // a connection but the rest of the transport resources remain valid (so the debugger helper thread won't // AV on a deallocated handle, which might happen if we simply called Shutdown()). - m_eState = SS_Closed; + SetSessionState(SS_Closed); } #else // RIGHT_SIDE_COMPILE @@ -245,14 +251,44 @@ void DbgTransportSession::CleanupTargetProcess() // returns true if the session opened within the time given (in milliseconds) and false otherwise. bool DbgTransportSession::WaitForSessionToOpen(DWORD dwTimeout) { - DWORD dwRet = WaitForSingleObject(m_hSessionOpenEvent, dwTimeout); - if (m_eState == SS_Closed) - return false; + int64_t start = minipal_lowres_ticks(); + uint32_t remaining = dwTimeout; + TransportLockHolder lock(m_sStateLock); - if (dwRet == WAIT_TIMEOUT) - DbgTransportLog(LC_Proxy, "DbgTransportSession::WaitForSessionToOpen(%u) timed out", dwTimeout); + while (m_eState != SS_Open && m_eState != SS_Closed) + { + minipal_condition_variable_result result = + minipal_condition_variable_wait(&m_sessionStateCondition, &m_sStateLock.GetMutex(), remaining); + if (result == MINIPAL_CONDITION_VARIABLE_FAILED) + { + return false; + } + + if (m_eState == SS_Open || m_eState == SS_Closed) + { + break; + } - return dwRet == WAIT_OBJECT_0; + if (result == MINIPAL_CONDITION_VARIABLE_TIMED_OUT) + { + DbgTransportLog(LC_Proxy, "DbgTransportSession::WaitForSessionToOpen(%u) timed out", dwTimeout); + return false; + } + + if (dwTimeout != INFINITE) + { + int64_t elapsed = minipal_lowres_ticks() - start; + if (elapsed >= static_cast(dwTimeout)) + { + DbgTransportLog(LC_Proxy, "DbgTransportSession::WaitForSessionToOpen(%u) timed out", dwTimeout); + return false; + } + + remaining = dwTimeout - static_cast(elapsed); + } + } + + return m_eState == SS_Open; } //--------------------------------------------------------------------------------------- @@ -343,14 +379,14 @@ HRESULT DbgTransportSession::SendDebugEvent(DebuggerIPCEvent * pEvent) // Retrieves the auto-reset handle which is signalled by the session each time a new event is received from // the other side. -HANDLE DbgTransportSession::GetIPCEventReadyEvent() +WaitEvent *DbgTransportSession::GetIPCEventReadyEvent() { return m_rghEventReadyEvent[IPCET_OldStyle]; } // Retrieves the auto-reset handle which is signalled by the session each time a new event (disguised as a // debug event) is received from the other side. -HANDLE DbgTransportSession::GetDebugEventReadyEvent() +WaitEvent *DbgTransportSession::GetDebugEventReadyEvent() { return m_rghEventReadyEvent[IPCET_DebugEvent]; } @@ -382,7 +418,7 @@ void DbgTransportSession::GetNextEvent(DebuggerIPCEvent *pEvent, DWORD cbEvent) // If there's at least one more valid event we can signal event ready now. if (m_cValidEventBuffers) { - SetEvent(m_rghEventReadyEvent[m_pEventBuffers[m_idxEventBufferHead].m_type]); + m_rghEventReadyEvent[m_pEventBuffers[m_idxEventBufferHead].m_type]->Set(); } } @@ -694,35 +730,33 @@ HRESULT DbgTransportSession::SendMessage(Message *pMessage, bool fWaitsForReply) HRESULT DbgTransportSession::SendRequestMessageAndWait(Message *pMessage) { // Allocate event to wait for reply on. - pMessage->m_hReplyEvent = CreateEvent(NULL, FALSE, FALSE, NULL); // Auto-reset, not signalled - if (pMessage->m_hReplyEvent == NULL) + pMessage->m_hReplyEvent = new (nothrow) WaitEvent(false); + if ((pMessage->m_hReplyEvent == nullptr) || !pMessage->m_hReplyEvent->IsValid()) + { + delete pMessage->m_hReplyEvent; + pMessage->m_hReplyEvent = nullptr; return E_OUTOFMEMORY; + } - // Duplicate the handle to the event. It's necessary to have two handles to the same event because - // both this thread and the message pumping thread may be trying to access the handle at the same - // time (e.g. closing the handle). So we make a duplicate handle. This thread is responsible for - // closing hReplyEvent (the local variable) whereas the message pumping thread is responsible for - // closing the handle on the message. - HANDLE hReplyEvent = NULL; - if (!DuplicateHandle(GetCurrentProcess(), - pMessage->m_hReplyEvent, - GetCurrentProcess(), - &hReplyEvent, - 0, // ignored since we are going to pass DUPLICATE_SAME_ACCESS - FALSE, - DUPLICATE_SAME_ACCESS)) + // Acquire a second owned reference to the event because both this thread and the message pumping + // thread may release their references at the same time. This thread owns hReplyEvent while the + // message pumping thread owns the reference on the message. + WaitEvent hReplyEvent(*pMessage->m_hReplyEvent); + if (!hReplyEvent.IsValid()) { - return HRESULT_FROM_GetLastError(); + delete pMessage->m_hReplyEvent; + pMessage->m_hReplyEvent = nullptr; + return E_FAIL; } // Send the request. HRESULT hr = SendMessage(pMessage, true); if (FAILED(hr)) { - // In this case, we need to close both handles since the message is never put into the send queue. + // Release both references since the message is never put into the send queue. // This thread is the only one who has access to the message. - CloseHandle(pMessage->m_hReplyEvent); - CloseHandle(hReplyEvent); + delete pMessage->m_hReplyEvent; + pMessage->m_hReplyEvent = nullptr; return hr; } @@ -732,25 +766,26 @@ HRESULT DbgTransportSession::SendRequestMessageAndWait(Message *pMessage) // Wait for a reply (by the time this event is signalled the message header will have been overwritten by // the reply and any output buffer provided will have been filled in). #if defined(RIGHT_SIDE_COMPILE) - HANDLE rgEvents[] = { hReplyEvent, m_hProcessExited }; + const WaitHandle *rgEvents[] = { &hReplyEvent, m_hProcessExited }; #else // !RIGHT_SIDE_COMPILE - HANDLE rgEvents[] = { hReplyEvent }; + const WaitHandle *rgEvents[] = { &hReplyEvent }; #endif // RIGHT_SIDE_COMPILE - DWORD dwResult = WaitForMultipleObjectsEx(sizeof(rgEvents)/sizeof(rgEvents[0]), rgEvents, FALSE, INFINITE, FALSE); + int32_t waitResult = WaitHandle::Wait( + rgEvents, + ARRAY_SIZE(rgEvents), + WaitHandle::Infinite); - if (dwResult == WAIT_OBJECT_0) + if (waitResult == 0) { // This is the normal case. The message pumping thread receives a reply from the debuggee process. // It signals the event to wake up this thread. - CloseHandle(hReplyEvent); - // Check whether the session aborted us due to a Shutdown(). if (pMessage->m_fAborted) return E_ABORT; } #if defined(RIGHT_SIDE_COMPILE) - else if (dwResult == (WAIT_OBJECT_0 + 1)) + else if (waitResult == 1) { // This is the complicated case. This thread wakes up because the debuggee process is terminated. // At the same time, the message pumping thread may be in the process of handling the reply message. @@ -769,17 +804,20 @@ HRESULT DbgTransportSession::SendRequestMessageAndWait(Message *pMessage) // Fortunately, in this case, we know the message pumping thread is going to signal the event. if (pOriginalMessage == NULL) { - WaitForSingleObject(hReplyEvent, INFINITE); + WaitHandle::Wait(hReplyEvent, WaitHandle::Infinite); + } + else + { + delete pOriginalMessage->m_hReplyEvent; + pOriginalMessage->m_hReplyEvent = nullptr; } - CloseHandle(hReplyEvent); return CORDBG_E_PROCESS_TERMINATED; } #endif // RIGHT_SIDE_COMPILE else { // Should never get here. - CloseHandle(hReplyEvent); UNREACHABLE(); } @@ -1033,7 +1071,7 @@ bool DbgTransportSession::ProcessReply(MessageHeader *pHeader) //--------------------------------------------------------------------------------------- // // Upon receiving a reply message, signal the event on the message to wake up the thread waiting for -// the reply message and close the handle to the event. +// the reply message and release its event reference. // // Arguments: // pMessage - the reply message to be processed @@ -1044,11 +1082,12 @@ void DbgTransportSession::SignalReplyEvent(Message * pMessage) // Make a local copy of the event handle. As soon as we signal the event, the thread blocked waiting on // the reply may wake up and trash the message. See code:DbgTransportSession::SendRequestMessageAndWait() // for more info. - HANDLE hReplyEvent = pMessage->m_hReplyEvent; - _ASSERTE(hReplyEvent != NULL); + WaitEvent *pReplyEvent = pMessage->m_hReplyEvent; + _ASSERTE(pReplyEvent != nullptr); + pMessage->m_hReplyEvent = nullptr; - SetEvent(hReplyEvent); - CloseHandle(hReplyEvent); + pReplyEvent->Set(); + delete pReplyEvent; } //--------------------------------------------------------------------------------------- @@ -1193,6 +1232,26 @@ void DbgTransportSession::InitSessionState() m_idxEventBufferTail = 0; } +void DbgTransportSession::SetSessionState(SessionState state) +{ + TransportLockHolder lock(m_sStateLock); + SetSessionStateUnderLock(state); +} + +void DbgTransportSession::SetSessionStateUnderLock(SessionState state) +{ + m_eState = state; + +#ifdef RIGHT_SIDE_COMPILE + if (state == SS_Open || state == SS_Closed) + { + bool result = minipal_condition_variable_broadcast(&m_sessionStateCondition); + _ASSERTE(result); + (void)result; + } +#endif // RIGHT_SIDE_COMPILE +} + // The entry point of the transport worker thread. This one's static, so we immediately dispatch to an // instance method version defined below for convenience in the implementation. DWORD WINAPI DbgTransportSession::TransportWorkerStatic(LPVOID pvContext) @@ -1218,7 +1277,7 @@ DWORD WINAPI DbgTransportSession::TransportWorkerStatic(LPVOID pvContext) } while (false) #define HANDLE_CRITICAL_ERROR() do { \ - m_eState = SS_Closed; \ + SetSessionState(SS_Closed); \ goto Shutdown; \ } while (false) @@ -1249,9 +1308,6 @@ void DbgTransportSession::TransportWorker() } #ifdef RIGHT_SIDE_COMPILE - // The session is definitely not open at this point. - ResetEvent(m_hSessionOpenEvent); - // On the right side we initiate the connection via Connect(). A failure is dealt with by waiting a // little while and retrying (the LS may take a little while to set up). If there's nobody listening // the debugger will eventually get bored waiting for us and shutdown the session, which will @@ -1493,16 +1549,11 @@ void DbgTransportSession::TransportWorker() if (m_eState == SS_Closed) break; else if (m_eState == SS_Opening) - m_eState = SS_Open; + SetSessionStateUnderLock(SS_Open); else _ASSERTE(!"Bad session state"); } // Leave m_sStateLock -#ifdef RIGHT_SIDE_COMPILE - // Signal any WaitForSessionToOpen() waiters that we've gotten to SS_Open. - SetEvent(m_hSessionOpenEvent); -#endif // RIGHT_SIDE_COMPILE - // We're ready to begin receiving normal incoming messages now. } else @@ -1624,7 +1675,7 @@ void DbgTransportSession::TransportWorker() // Finished processing queued sends. We can transition to the SS_Open state now as long as there // wasn't a send failure or an asynchronous Shutdown(). if (m_eState == SS_Resync) - m_eState = SS_Open; + SetSessionStateUnderLock(SS_Open); else if (m_eState == SS_Closed) break; else if (m_eState == SS_Resync_NC) @@ -1741,14 +1792,14 @@ void DbgTransportSession::TransportWorker() case MT_SessionReject: case MT_SessionResync: // Illegal messages at this time, fail the transport entirely. - m_eState = SS_Closed; + SetSessionState(SS_Closed); break; case MT_SessionClose: // Close is legal on the LS and transitions to the SS_Opening_NC state. It's illegal on the RS // and should shutdown the transport. #ifdef RIGHT_SIDE_COMPILE - m_eState = SS_Closed; + SetSessionState(SS_Closed); break; #else // RIGHT_SIDE_COMPILE // We need to do some state cleanup here, since when we reform a connection (if ever, it will @@ -1759,7 +1810,7 @@ void DbgTransportSession::TransportWorker() // Check we're still in a good state before a clean restart. if (m_eState != SS_Open) { - m_eState = SS_Closed; + SetSessionStateUnderLock(SS_Closed); break; } @@ -1868,7 +1919,7 @@ void DbgTransportSession::TransportWorker() // If we just added the first valid event then wake up the client so they can call // GetNextEvent(). if (m_cValidEventBuffers == 1) - SetEvent(m_rghEventReadyEvent[m_pEventBuffers[idxCurrentEvent].m_type]); + m_rghEventReadyEvent[m_pEventBuffers[idxCurrentEvent].m_type]->Set(); } } break; @@ -2031,11 +2082,6 @@ void DbgTransportSession::TransportWorker() _ASSERTE(m_eState == SS_Closed); -#ifdef RIGHT_SIDE_COMPILE - // The session is definitely not open at this point. - ResetEvent(m_hSessionOpenEvent); -#endif // RIGHT_SIDE_COMPILE - // Close the connection if we haven't done so already. if (m_channel != NULL) m_channel->CloseConnection(); diff --git a/src/coreclr/inc/cordebug.idl b/src/coreclr/inc/cordebug.idl index 5dfcb037abc5a1..9204c3944427e0 100644 --- a/src/coreclr/inc/cordebug.idl +++ b/src/coreclr/inc/cordebug.idl @@ -2690,9 +2690,10 @@ interface ICorDebugProcess : ICorDebugController HRESULT GetID([out] DWORD *pdwProcessId); /* - * GetHandle returns a handle to the process. This handle is owned - * by the debugging API; the debugger should duplicate it before - * using it. + * GetHandle returns a handle to the process on Windows. This handle + * is owned by the debugging API; the debugger should duplicate it + * before using it. On Unix, this method returns an opaque sentinel + * that must not be used as a native process handle. */ HRESULT GetHandle([out] HPROCESS *phProcessHandle); diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.Unix.cs index 5232c73bd31bc5..1de0d19adb8d8b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.Unix.cs @@ -36,8 +36,8 @@ namespace System.Threading /// interruptible /// - is used for the process-wide lock /// - is the main system dependency of the wait subsystem, and all waits are done through - /// it. It is backed by a C++ equivalent in CoreLib.Native's pal_threading.*, which wraps a pthread mutex/condition - /// pair. Each thread has an instance in , which is used to synchronize the + /// it. It is backed by a native implementation in System.Native that uses a minipal mutex/condition pair. + /// Each thread has an instance in , which is used to synchronize the /// thread's wait state and for waiting. also uses an instance of /// for waiting. /// @@ -107,7 +107,7 @@ namespace System.Threading /// - Most of the wait subsystem is written in C#, so there is no initially required p/invoke /// - , used by the process-wide lock , uses interlocked operations to /// acquire and release the lock when there is no need to wait or to release a waiter. This is significantly faster than - /// using as a lock, which uses pthread mutex functionality through p/invoke. The lock is + /// using as a lock, which uses native synchronization through p/invoke. The lock is /// typically not held for very long, especially since allocations inside the lock will be rare. /// - Since provides mutual exclusion for the states of all s in the /// process, any operation that does not involve waiting or releasing a wait can occur with minimal p/invokes diff --git a/src/native/libs/System.Native/pal_threading.c b/src/native/libs/System.Native/pal_threading.c index eb64103d8e4766..bc25b29016e577 100644 --- a/src/native/libs/System.Native/pal_threading.c +++ b/src/native/libs/System.Native/pal_threading.c @@ -12,17 +12,13 @@ #include #include #include -#include +#include +#include #include #if HAVE_SCHED_GETCPU #include #endif -#if defined(TARGET_OSX) -// So we can use the declaration of pthread_cond_timedwait_relative_np -#undef _XOPEN_SOURCE -#endif - #if defined(TARGET_LINUX) #include /* Definition of FUTEX_* constants */ #include /* Definition of SYS_* constants */ @@ -41,8 +37,8 @@ struct LowLevelMonitor { - pthread_mutex_t Mutex; - pthread_cond_t Condition; + minipal_nonrecursive_mutex Mutex; + minipal_condition_variable Condition; #ifdef DEBUG bool IsLocked; #endif @@ -67,42 +63,17 @@ LowLevelMonitor* SystemNative_LowLevelMonitor_Create(void) return NULL; } - int error; - - error = pthread_mutex_init(&monitor->Mutex, NULL); - if (error != 0) + if (!minipal_nonrecursive_mutex_init(&monitor->Mutex)) { free(monitor); return NULL; } -#if HAVE_PTHREAD_CONDATTR_SETCLOCK - pthread_condattr_t conditionAttributes; - error = pthread_condattr_init(&conditionAttributes); - if (error != 0) - { - goto mutex_destroy; - } - - error = pthread_condattr_setclock(&conditionAttributes, CLOCK_MONOTONIC); - if (error != 0) + if (!minipal_condition_variable_init(&monitor->Condition)) { - error = pthread_condattr_destroy(&conditionAttributes); - assert(error == 0); - goto mutex_destroy; - } - - error = pthread_cond_init(&monitor->Condition, &conditionAttributes); - - int condAttrDestroyError; - condAttrDestroyError = pthread_condattr_destroy(&conditionAttributes); - assert(condAttrDestroyError == 0); -#else - error = pthread_cond_init(&monitor->Condition, NULL); -#endif - if (error != 0) - { - goto mutex_destroy; + minipal_nonrecursive_mutex_destroy(&monitor->Mutex); + free(monitor); + return NULL; } #ifdef DEBUG @@ -110,25 +81,14 @@ LowLevelMonitor* SystemNative_LowLevelMonitor_Create(void) #endif return monitor; - -mutex_destroy: - error = pthread_mutex_destroy(&monitor->Mutex); - assert(error == 0); - free(monitor); - return NULL; } void SystemNative_LowLevelMonitor_Destroy(LowLevelMonitor* monitor) { assert(monitor != NULL); - int error; - - error = pthread_cond_destroy(&monitor->Condition); - assert(error == 0); - - error = pthread_mutex_destroy(&monitor->Mutex); - assert(error == 0); + minipal_condition_variable_destroy(&monitor->Condition); + minipal_nonrecursive_mutex_destroy(&monitor->Mutex); free(monitor); } @@ -137,10 +97,7 @@ void SystemNative_LowLevelMonitor_Acquire(LowLevelMonitor* monitor) { assert(monitor != NULL); - int error; - - error = pthread_mutex_lock(&monitor->Mutex); - assert(error == 0); + minipal_nonrecursive_mutex_enter(&monitor->Mutex); SetIsLocked(monitor, true); } @@ -151,10 +108,7 @@ void SystemNative_LowLevelMonitor_Release(LowLevelMonitor* monitor) SetIsLocked(monitor, false); - int error; - - error = pthread_mutex_unlock(&monitor->Mutex); - assert(error == 0); + minipal_nonrecursive_mutex_leave(&monitor->Mutex); } void SystemNative_LowLevelMonitor_Wait(LowLevelMonitor* monitor) @@ -163,10 +117,12 @@ void SystemNative_LowLevelMonitor_Wait(LowLevelMonitor* monitor) SetIsLocked(monitor, false); - int error; - - error = pthread_cond_wait(&monitor->Condition, &monitor->Mutex); - assert(error == 0); + minipal_condition_variable_result result = + minipal_condition_variable_wait_nonrecursive( + &monitor->Condition, + &monitor->Mutex, + MINIPAL_CONDITION_VARIABLE_INFINITE); + assert(result == MINIPAL_CONDITION_VARIABLE_SIGNALED); SetIsLocked(monitor, true); } @@ -177,55 +133,30 @@ int32_t SystemNative_LowLevelMonitor_TimedWait(LowLevelMonitor *monitor, int32_t SetIsLocked(monitor, false); - int error; - - // Calculate the time at which a timeout should occur, and wait. Older versions of OSX don't support clock_gettime with - // CLOCK_MONOTONIC, so we instead compute the relative timeout duration, and use a relative variant of the timed wait. - struct timespec timeoutTimeSpec; -#if HAVE_CLOCK_GETTIME_NSEC_NP - timeoutTimeSpec.tv_sec = timeoutMilliseconds / 1000; - timeoutTimeSpec.tv_nsec = (timeoutMilliseconds % 1000) * 1000 * 1000; - - error = pthread_cond_timedwait_relative_np(&monitor->Condition, &monitor->Mutex, &timeoutTimeSpec); -#else -#if HAVE_PTHREAD_CONDATTR_SETCLOCK - error = clock_gettime(CLOCK_MONOTONIC, &timeoutTimeSpec); - assert(error == 0); -#else - struct timeval tv; - - error = gettimeofday(&tv, NULL); - assert(error == 0); - - timeoutTimeSpec.tv_sec = tv.tv_sec; - timeoutTimeSpec.tv_nsec = tv.tv_usec * 1000; -#endif - uint64_t nanoseconds = (uint64_t)timeoutMilliseconds * 1000 * 1000 + (uint64_t)timeoutTimeSpec.tv_nsec; - timeoutTimeSpec.tv_sec += nanoseconds / (1000 * 1000 * 1000); - timeoutTimeSpec.tv_nsec = nanoseconds % (1000 * 1000 * 1000); - - error = pthread_cond_timedwait(&monitor->Condition, &monitor->Mutex, &timeoutTimeSpec); -#endif - assert(error == 0 || error == ETIMEDOUT); + minipal_condition_variable_result result = + minipal_condition_variable_wait_nonrecursive( + &monitor->Condition, + &monitor->Mutex, + (uint32_t)timeoutMilliseconds); + assert( + result == MINIPAL_CONDITION_VARIABLE_SIGNALED || + result == MINIPAL_CONDITION_VARIABLE_TIMED_OUT); SetIsLocked(monitor, true); - return error == 0; + return result == MINIPAL_CONDITION_VARIABLE_SIGNALED; } void SystemNative_LowLevelMonitor_Signal_Release(LowLevelMonitor* monitor) { assert(monitor != NULL); - int error; - - error = pthread_cond_signal(&monitor->Condition); - assert(error == 0); + bool result = minipal_condition_variable_signal(&monitor->Condition); + assert(result); SetIsLocked(monitor, false); - error = pthread_mutex_unlock(&monitor->Mutex); - assert(error == 0); + minipal_nonrecursive_mutex_leave(&monitor->Mutex); } #if defined(TARGET_LINUX) diff --git a/src/native/minipal/CMakeLists.txt b/src/native/minipal/CMakeLists.txt index 53960991f53080..309495bc96e307 100644 --- a/src/native/minipal/CMakeLists.txt +++ b/src/native/minipal/CMakeLists.txt @@ -3,6 +3,7 @@ set(CMAKE_INCLUDE_CURRENT_DIR OFF) include(configure.cmake) set(SOURCES + conditionvariable.c cpufeatures.c descriptorlimit.c entrypoints.c diff --git a/src/native/minipal/conditionvariable.c b/src/native/minipal/conditionvariable.c new file mode 100644 index 00000000000000..1cdcb83ead36ab --- /dev/null +++ b/src/native/minipal/conditionvariable.c @@ -0,0 +1,183 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#include +#include +#include + +#include + +#include "minipalconfig.h" + +#define NANOSECONDS_PER_MILLISECOND 1000000 +#define NANOSECONDS_PER_SECOND 1000000000 + +#if !defined(HOST_WINDOWS) && !HAVE_CLOCK_GETTIME_NSEC_NP +static bool GetDeadline(clockid_t clock, uint32_t timeoutMilliseconds, struct timespec* deadline) +{ + if (clock_gettime(clock, deadline) != 0) + { + return false; + } + + uint64_t nanoseconds = + (uint64_t)deadline->tv_nsec + + (uint64_t)timeoutMilliseconds * NANOSECONDS_PER_MILLISECOND; + deadline->tv_sec += (time_t)(nanoseconds / NANOSECONDS_PER_SECOND); + deadline->tv_nsec = (long)(nanoseconds % NANOSECONDS_PER_SECOND); + return true; +} +#endif // !HOST_WINDOWS && !HAVE_CLOCK_GETTIME_NSEC_NP + +bool minipal_condition_variable_init(minipal_condition_variable* condition) +{ + assert(condition != NULL); +#ifdef HOST_WINDOWS + InitializeConditionVariable(&condition->_impl); + return true; +#else + pthread_condattr_t attributes; + if (pthread_condattr_init(&attributes) != 0) + { + return false; + } + +#if HAVE_PTHREAD_CONDATTR_SETCLOCK + if (pthread_condattr_setclock(&attributes, CLOCK_MONOTONIC) != 0) + { + int error = pthread_condattr_destroy(&attributes); + assert(error == 0); + (void)error; + return false; + } +#endif // HAVE_PTHREAD_CONDATTR_SETCLOCK + + bool success = pthread_cond_init(&condition->_impl, &attributes) == 0; + int error = pthread_condattr_destroy(&attributes); + assert(error == 0); + (void)error; + return success; +#endif // HOST_WINDOWS +} + +void minipal_condition_variable_destroy(minipal_condition_variable* condition) +{ + assert(condition != NULL); +#ifndef HOST_WINDOWS + int error = pthread_cond_destroy(&condition->_impl); + assert(error == 0); + (void)error; +#else + (void)condition; +#endif // !HOST_WINDOWS +} + +bool minipal_condition_variable_broadcast(minipal_condition_variable* condition) +{ + assert(condition != NULL); +#ifdef HOST_WINDOWS + WakeAllConditionVariable(&condition->_impl); + return true; +#else + return pthread_cond_broadcast(&condition->_impl) == 0; +#endif // HOST_WINDOWS +} + +bool minipal_condition_variable_signal(minipal_condition_variable* condition) +{ + assert(condition != NULL); +#ifdef HOST_WINDOWS + WakeConditionVariable(&condition->_impl); + return true; +#else + return pthread_cond_signal(&condition->_impl) == 0; +#endif // HOST_WINDOWS +} + +#ifndef HOST_WINDOWS +static minipal_condition_variable_result minipal_condition_variable_wait_pthread( + minipal_condition_variable* condition, + pthread_mutex_t* mutex, + uint32_t timeoutMilliseconds) +{ + int error; + if (timeoutMilliseconds == MINIPAL_CONDITION_VARIABLE_INFINITE) + { + error = pthread_cond_wait(&condition->_impl, mutex); + } + else + { + struct timespec timeout; +#if HAVE_CLOCK_GETTIME_NSEC_NP + uint64_t nanoseconds = (uint64_t)timeoutMilliseconds * NANOSECONDS_PER_MILLISECOND; + timeout.tv_sec = (time_t)(nanoseconds / NANOSECONDS_PER_SECOND); + timeout.tv_nsec = (long)(nanoseconds % NANOSECONDS_PER_SECOND); + error = pthread_cond_timedwait_relative_np(&condition->_impl, mutex, &timeout); +#else +#if HAVE_PTHREAD_CONDATTR_SETCLOCK + const clockid_t waitClock = CLOCK_MONOTONIC; +#else + const clockid_t waitClock = CLOCK_REALTIME; +#endif // HAVE_PTHREAD_CONDATTR_SETCLOCK + if (!GetDeadline(waitClock, timeoutMilliseconds, &timeout)) + { + return MINIPAL_CONDITION_VARIABLE_FAILED; + } + error = pthread_cond_timedwait(&condition->_impl, mutex, &timeout); +#endif // HAVE_CLOCK_GETTIME_NSEC_NP + } + + if (error == 0) + { + return MINIPAL_CONDITION_VARIABLE_SIGNALED; + } + + return error == ETIMEDOUT + ? MINIPAL_CONDITION_VARIABLE_TIMED_OUT + : MINIPAL_CONDITION_VARIABLE_FAILED; +} +#endif // !HOST_WINDOWS + +minipal_condition_variable_result minipal_condition_variable_wait( + minipal_condition_variable* condition, + minipal_mutex* mutex, + uint32_t timeoutMilliseconds) +{ + assert(condition != NULL); + assert(mutex != NULL); + +#ifdef HOST_WINDOWS + if (SleepConditionVariableCS(&condition->_impl, &mutex->_impl, timeoutMilliseconds) != FALSE) + { + return MINIPAL_CONDITION_VARIABLE_SIGNALED; + } + + return GetLastError() == ERROR_TIMEOUT + ? MINIPAL_CONDITION_VARIABLE_TIMED_OUT + : MINIPAL_CONDITION_VARIABLE_FAILED; +#else + return minipal_condition_variable_wait_pthread(condition, &mutex->_impl, timeoutMilliseconds); +#endif // HOST_WINDOWS +} + +minipal_condition_variable_result minipal_condition_variable_wait_nonrecursive( + minipal_condition_variable* condition, + minipal_nonrecursive_mutex* mutex, + uint32_t timeoutMilliseconds) +{ + assert(condition != NULL); + assert(mutex != NULL); + +#ifdef HOST_WINDOWS + if (SleepConditionVariableSRW(&condition->_impl, &mutex->_impl, timeoutMilliseconds, 0) != FALSE) + { + return MINIPAL_CONDITION_VARIABLE_SIGNALED; + } + + return GetLastError() == ERROR_TIMEOUT + ? MINIPAL_CONDITION_VARIABLE_TIMED_OUT + : MINIPAL_CONDITION_VARIABLE_FAILED; +#else + return minipal_condition_variable_wait_pthread(condition, &mutex->_impl, timeoutMilliseconds); +#endif // HOST_WINDOWS +} diff --git a/src/native/minipal/conditionvariable.h b/src/native/minipal/conditionvariable.h new file mode 100644 index 00000000000000..dd8d7ed2b206df --- /dev/null +++ b/src/native/minipal/conditionvariable.h @@ -0,0 +1,70 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#ifndef HAVE_MINIPAL_CONDITION_VARIABLE_H +#define HAVE_MINIPAL_CONDITION_VARIABLE_H + +#include +#include + +#include "mutex.h" + +#ifdef HOST_WINDOWS +#include +typedef CONDITION_VARIABLE MINIPAL_CONDITION_VARIABLE_IMPL; +#else +#include +typedef pthread_cond_t MINIPAL_CONDITION_VARIABLE_IMPL; +#endif // HOST_WINDOWS + +#ifdef __cplusplus +extern "C" +{ +#endif // __cplusplus + +typedef struct _minipal_condition_variable +{ + MINIPAL_CONDITION_VARIABLE_IMPL _impl; +} minipal_condition_variable; + +typedef enum _minipal_condition_variable_result +{ + MINIPAL_CONDITION_VARIABLE_SIGNALED, + MINIPAL_CONDITION_VARIABLE_TIMED_OUT, + MINIPAL_CONDITION_VARIABLE_FAILED, +} minipal_condition_variable_result; + +#define MINIPAL_CONDITION_VARIABLE_INFINITE UINT32_MAX + +// Initialize the condition variable. +bool minipal_condition_variable_init(minipal_condition_variable* condition); + +// Destroy the condition variable. No threads may be waiting. +void minipal_condition_variable_destroy(minipal_condition_variable* condition); + +// Wake all threads waiting on the condition variable. +bool minipal_condition_variable_broadcast(minipal_condition_variable* condition); + +// Wake one thread waiting on the condition variable. +bool minipal_condition_variable_signal(minipal_condition_variable* condition); + +// Atomically release the entered mutex and wait, then reacquire it before returning. +// The calling thread must have entered the mutex exactly once. +// The caller must recheck its predicate after every signaled result because wakes may be spurious. +minipal_condition_variable_result minipal_condition_variable_wait( + minipal_condition_variable* condition, + minipal_mutex* mutex, + uint32_t timeoutMilliseconds); + +// Atomically release the entered non-recursive mutex and wait, then reacquire it before returning. +// The caller must recheck its predicate after every signaled result because wakes may be spurious. +minipal_condition_variable_result minipal_condition_variable_wait_nonrecursive( + minipal_condition_variable* condition, + minipal_nonrecursive_mutex* mutex, + uint32_t timeoutMilliseconds); + +#ifdef __cplusplus +} +#endif // __cplusplus + +#endif // HAVE_MINIPAL_CONDITION_VARIABLE_H diff --git a/src/native/minipal/configure.cmake b/src/native/minipal/configure.cmake index 00be7c33163c69..acf3892985996c 100644 --- a/src/native/minipal/configure.cmake +++ b/src/native/minipal/configure.cmake @@ -1,5 +1,6 @@ include(CheckFunctionExists) include(CheckIncludeFiles) +include(CheckLibraryExists) include(CheckSymbolExists) check_include_files("windows.h;bcrypt.h" HAVE_BCRYPT_H) @@ -19,6 +20,19 @@ check_symbol_exists(O_CLOEXEC fcntl.h HAVE_O_CLOEXEC) check_symbol_exists(CLOCK_MONOTONIC_COARSE time.h HAVE_CLOCK_MONOTONIC_COARSE) check_symbol_exists(clock_gettime_nsec_np time.h HAVE_CLOCK_GETTIME_NSEC_NP) +if(CLR_CMAKE_HOST_UNIX) + check_library_exists(pthread pthread_create "" HAVE_LIBPTHREAD) + check_library_exists(c pthread_create "" HAVE_PTHREAD_IN_LIBC) + if(HAVE_LIBPTHREAD) + set(PTHREAD_LIBRARY pthread) + elseif(HAVE_PTHREAD_IN_LIBC) + set(PTHREAD_LIBRARY c) + endif() + if(PTHREAD_LIBRARY) + check_library_exists(${PTHREAD_LIBRARY} pthread_condattr_setclock "" HAVE_PTHREAD_CONDATTR_SETCLOCK) + endif() +endif() + if(CMAKE_C_BYTE_ORDER STREQUAL "BIG_ENDIAN") set(BIGENDIAN 1) endif() diff --git a/src/native/minipal/minipalconfig.h.in b/src/native/minipal/minipalconfig.h.in index f78a66a7fcd852..de01c3ece7bfaf 100644 --- a/src/native/minipal/minipalconfig.h.in +++ b/src/native/minipal/minipalconfig.h.in @@ -13,6 +13,7 @@ #cmakedefine01 HAVE_ELF_AUX_INFO #cmakedefine01 HAVE_CLOCK_MONOTONIC_COARSE #cmakedefine01 HAVE_CLOCK_GETTIME_NSEC_NP +#cmakedefine01 HAVE_PTHREAD_CONDATTR_SETCLOCK #cmakedefine01 BIGENDIAN #cmakedefine01 HAVE_BCRYPT_H #cmakedefine01 HAVE_FSYNC diff --git a/src/native/minipal/mutex.c b/src/native/minipal/mutex.c index 8d34ca95f5a9ae..0fafc8190d4c2a 100644 --- a/src/native/minipal/mutex.c +++ b/src/native/minipal/mutex.c @@ -27,6 +27,28 @@ bool minipal_mutex_init(minipal_mutex* mtx) #endif // HOST_WINDOWS } +bool minipal_nonrecursive_mutex_init(minipal_nonrecursive_mutex* mtx) +{ + assert(mtx != NULL); +#ifdef HOST_WINDOWS + InitializeSRWLock(&mtx->_impl); + return true; +#else + pthread_mutexattr_t mutexAttributes; + int st = pthread_mutexattr_init(&mutexAttributes); + if (st != 0) + return false; + + st = pthread_mutexattr_settype(&mutexAttributes, PTHREAD_MUTEX_NORMAL); + if (st == 0) + st = pthread_mutex_init(&mtx->_impl, &mutexAttributes); + + pthread_mutexattr_destroy(&mutexAttributes); + + return (st == 0); +#endif // HOST_WINDOWS +} + void minipal_mutex_destroy(minipal_mutex* mtx) { assert(mtx != NULL); @@ -43,6 +65,20 @@ void minipal_mutex_destroy(minipal_mutex* mtx) #endif // _DEBUG } +void minipal_nonrecursive_mutex_destroy(minipal_nonrecursive_mutex* mtx) +{ + assert(mtx != NULL); +#ifndef HOST_WINDOWS + int st = pthread_mutex_destroy(&mtx->_impl); + assert(st == 0); + (void)st; +#endif // !HOST_WINDOWS + +#ifdef _DEBUG + memset(mtx, 0, sizeof(*mtx)); +#endif // _DEBUG +} + void minipal_mutex_enter(minipal_mutex* mtx) { assert(mtx != NULL); @@ -55,6 +91,18 @@ void minipal_mutex_enter(minipal_mutex* mtx) #endif // HOST_WINDOWS } +void minipal_nonrecursive_mutex_enter(minipal_nonrecursive_mutex* mtx) +{ + assert(mtx != NULL); +#ifdef HOST_WINDOWS + AcquireSRWLockExclusive(&mtx->_impl); +#else + int st = pthread_mutex_lock(&mtx->_impl); + assert(st == 0); + (void)st; +#endif // HOST_WINDOWS +} + void minipal_mutex_leave(minipal_mutex* mtx) { assert(mtx != NULL); @@ -66,3 +114,15 @@ void minipal_mutex_leave(minipal_mutex* mtx) (void)st; #endif // HOST_WINDOWS } + +void minipal_nonrecursive_mutex_leave(minipal_nonrecursive_mutex* mtx) +{ + assert(mtx != NULL); +#ifdef HOST_WINDOWS + ReleaseSRWLockExclusive(&mtx->_impl); +#else + int st = pthread_mutex_unlock(&mtx->_impl); + assert(st == 0); + (void)st; +#endif // HOST_WINDOWS +} diff --git a/src/native/minipal/mutex.h b/src/native/minipal/mutex.h index df94bac6f1bd33..cc7002cd509bb4 100644 --- a/src/native/minipal/mutex.h +++ b/src/native/minipal/mutex.h @@ -9,9 +9,11 @@ #ifdef HOST_WINDOWS #include typedef CRITICAL_SECTION MINIPAL_MUTEX_IMPL; +typedef SRWLOCK MINIPAL_NONRECURSIVE_MUTEX_IMPL; #else // !HOST_WINDOWS #include typedef pthread_mutex_t MINIPAL_MUTEX_IMPL; +typedef pthread_mutex_t MINIPAL_NONRECURSIVE_MUTEX_IMPL; #endif // HOST_WINDOWS #ifdef __cplusplus @@ -24,19 +26,36 @@ typedef struct _minipal_mutex MINIPAL_MUTEX_IMPL _impl; } minipal_mutex; +typedef struct _minipal_nonrecursive_mutex +{ + MINIPAL_NONRECURSIVE_MUTEX_IMPL _impl; +} minipal_nonrecursive_mutex; + // Initialize the mutex. bool minipal_mutex_init(minipal_mutex* mt); +// Initialize a non-recursive mutex. +bool minipal_nonrecursive_mutex_init(minipal_nonrecursive_mutex* mt); + // Destroy the mutex. void minipal_mutex_destroy(minipal_mutex* mt); +// Destroy the non-recursive mutex. +void minipal_nonrecursive_mutex_destroy(minipal_nonrecursive_mutex* mt); + // Enter the mutex. Blocks until the mutex can be entered. // Recursive enters are allowed. void minipal_mutex_enter(minipal_mutex* mt); +// Enter the non-recursive mutex. Blocks until the mutex can be entered. +void minipal_nonrecursive_mutex_enter(minipal_nonrecursive_mutex* mt); + // Leave the mutex. void minipal_mutex_leave(minipal_mutex* mt); +// Leave the non-recursive mutex. +void minipal_nonrecursive_mutex_leave(minipal_nonrecursive_mutex* mt); + #ifdef __cplusplus } #endif // __cplusplus