diff --git a/src/arch/Threads/Threads_Win32.cpp b/src/arch/Threads/Threads_Win32.cpp index 48da0dee0c..52d8ba970e 100644 --- a/src/arch/Threads/Threads_Win32.cpp +++ b/src/arch/Threads/Threads_Win32.cpp @@ -6,37 +6,46 @@ #include "archutils/Win32/ErrorStrings.h" #include +#include +#include -const int MAX_THREADS=128; +const int MAX_THREADS = 128; + +static std::unique_ptr g_pThreadIdMutex; +static bool g_ThreadIdMutexInitialized = false; +static std::mutex g_ThreadIdMutexInitLock; -static MutexImpl_Win32 *g_pThreadIdMutex = nullptr; static void InitThreadIdMutex() { - if( g_pThreadIdMutex != nullptr ) + std::lock_guard lock(g_ThreadIdMutexInitLock); + if (g_ThreadIdMutexInitialized) return; - g_pThreadIdMutex = new MutexImpl_Win32(nullptr); + + std::unique_ptr temp = std::make_unique(nullptr); + g_ThreadIdMutexInitialized = true; + + g_pThreadIdMutex = std::move(temp); } static uint64_t g_ThreadIds[MAX_THREADS]; static HANDLE g_ThreadHandles[MAX_THREADS]; +static std::mutex g_ThreadDataMutex; -HANDLE Win32ThreadIdToHandle( uint64_t iID ) +HANDLE Win32ThreadIdToHandle(uint64_t iID) { - for( int i = 0; i < MAX_THREADS; ++i ) + std::lock_guard lock(g_ThreadDataMutex); + for (int i = 0; i < MAX_THREADS; ++i) { - if( g_ThreadIds[i] == iID ) + if (g_ThreadIds[i] == iID) return g_ThreadHandles[i]; } return nullptr; } -void ThreadImpl_Win32::Halt( bool Kill ) +void ThreadImpl_Win32::Halt(bool /*Kill*/) { - if( Kill ) - TerminateThread( ThreadHandle, 0 ); - else - SuspendThread( ThreadHandle ); + SuspendThread(ThreadHandle); } void ThreadImpl_Win32::Resume() @@ -46,7 +55,7 @@ void ThreadImpl_Win32::Resume() uint64_t ThreadImpl_Win32::GetThreadId() const { - return (uint64_t) ThreadId; + return static_cast(ThreadId); } int ThreadImpl_Win32::Wait() @@ -82,10 +91,16 @@ static void SetThreadName( DWORD dwThreadID, LPCTSTR szThreadName ) info.dwThreadID = dwThreadID; info.dwFlags = 0; - __try { - RaiseException(MS_VC_EXCEPTION, 0, sizeof(info) / sizeof(DWORD), (ULONG_PTR *)&info); - } __except (EXCEPTION_CONTINUE_EXECUTION) { +#ifdef _DEBUG + __try + { + RaiseException(MS_VC_EXCEPTION, 0, sizeof(info) / sizeof(DWORD), (ULONG_PTR*)&info); } + __except (EXCEPTION_CONTINUE_EXECUTION) // NOTE(sukibaby): compiler suggests trying EXCEPTION_EXECUTE_HANDLER to avoid possible infinite loop. + { + } +#endif + #elif defined(__GNUC__) pthread_setname_np(pthread_self(), szThreadName); #endif @@ -99,7 +114,8 @@ static DWORD WINAPI StartThread( LPVOID pData ) DWORD ret = static_cast(pThis->m_pFunc(pThis->m_pData)); - for( int i = 0; i < MAX_THREADS; ++i ) + std::lock_guard lock(g_ThreadDataMutex); + for (int i = 0; i < MAX_THREADS; ++i) { if( g_ThreadIds[i] == RageThread::GetCurrentThreadID() ) { @@ -133,7 +149,7 @@ static int GetOpenSlot( uint64_t iID ) ThreadImpl *MakeThisThread() { - ThreadImpl_Win32 *thread = new ThreadImpl_Win32; + std::unique_ptr thread = std::make_unique(); SetThreadName( GetCurrentThreadId(), RageThread::GetCurrentThreadName() ); @@ -143,9 +159,6 @@ ThreadImpl *MakeThisThread() if( !ret ) { -// LOG->Warn( werr_ssprintf( GetLastError(), "DuplicateHandle(%p, %p) failed", -// CurProc, GetCurrentThread() ) ); - thread->ThreadHandle = nullptr; } @@ -154,18 +167,18 @@ ThreadImpl *MakeThisThread() int slot = GetOpenSlot( GetCurrentThreadId() ); g_ThreadHandles[slot] = thread->ThreadHandle; - return thread; + return thread.release(); } ThreadImpl *MakeThread( int (*pFunc)(void *pData), void *pData, uint64_t *piThreadID ) { - ThreadImpl_Win32 *thread = new ThreadImpl_Win32; + std::unique_ptr thread = std::make_unique(); thread->m_pFunc = pFunc; thread->m_pData = pData; - thread->ThreadHandle = CreateThread( nullptr, 0, &StartThread, thread, CREATE_SUSPENDED, &thread->ThreadId ); - *piThreadID = (uint64_t) thread->ThreadId; - ASSERT_M( thread->ThreadHandle != nullptr, ssprintf("%s", werr_ssprintf(GetLastError(), "CreateThread").c_str() ) ); + thread->ThreadHandle = CreateThread(nullptr, 0, &StartThread, thread.get(), CREATE_SUSPENDED, &thread->ThreadId); + *piThreadID = static_cast(thread->ThreadId); + ASSERT_M(thread->ThreadHandle != nullptr, ssprintf("%s", werr_ssprintf(GetLastError(), "CreateThread").c_str())); int slot = GetOpenSlot( thread->ThreadId ); g_ThreadHandles[slot] = thread->ThreadHandle; @@ -173,7 +186,7 @@ ThreadImpl *MakeThread( int (*pFunc)(void *pData), void *pData, uint64_t *piThre int iRet = ResumeThread( thread->ThreadHandle ); ASSERT_M( iRet == 1, ssprintf("%s", werr_ssprintf(GetLastError(), "ResumeThread").c_str() ) ); - return thread; + return thread.release(); } @@ -189,6 +202,12 @@ MutexImpl_Win32::~MutexImpl_Win32() CloseHandle( mutex ); } +/* NOTE(sukibaby): the function name here is a bit misleading. + * It provides additional error handling and assertions which + * are not present in WaitForSingleObject. + * It also consolidates some switch case logic we need to use + * mutliple times within this file. + * For these reasons I've left this function alone. */ static bool SimpleWaitForSingleObject( HANDLE h, DWORD ms ) { ASSERT( h != nullptr ); @@ -203,35 +222,34 @@ static bool SimpleWaitForSingleObject( HANDLE h, DWORD ms ) return false; case WAIT_ABANDONED: - // The docs aren't particular about what this does, but it should never happen. - FAIL_M( "WAIT_ABANDONED" ); + FAIL_M("WAIT_ABANDONED"); case WAIT_FAILED: - FAIL_M( werr_ssprintf(GetLastError(), "WaitForSingleObject") ); + FAIL_M(werr_ssprintf(GetLastError(), "WaitForSingleObject")); default: - FAIL_M( "unknown" ); + FAIL_M("unknown"); } } bool MutexImpl_Win32::Lock() +{ + DWORD dwWaitResult = WaitForSingleObject(mutex, INFINITE); + switch (dwWaitResult) { - DWORD dwWaitResult = WaitForSingleObject(mutex, INFINITE); - switch (dwWaitResult) - { - case WAIT_OBJECT_0: - return true; - - case WAIT_TIMEOUT: - return false; + case WAIT_OBJECT_0: + return true; - case WAIT_ABANDONED: - return false; + case WAIT_TIMEOUT: + return false; - default: - FAIL_M( "WaitForSingleObject failed in a way that shouldn't have been possible" ); - } + case WAIT_ABANDONED: + return false; + + default: + FAIL_M("WaitForSingleObject failed in a way that shouldn't have been possible"); } +} bool MutexImpl_Win32::TryLock() { @@ -282,37 +300,6 @@ EventImpl_Win32::~EventImpl_Win32() CloseHandle( m_WaitersDone ); } -static bool PortableSignalObjectAndWait( HANDLE hObjectToSignal, HANDLE hObjectToWaitOn, bool bFirstParamIsMutex, unsigned iMilliseconds = INFINITE ) -{ - if( bFirstParamIsMutex ) - { - const bool bRet = !!ReleaseMutex( hObjectToSignal ); - if( !bRet ) - sm_crash( werr_ssprintf( GetLastError(), "ReleaseMutex failed" ) ); - } - else - { - SetEvent( hObjectToSignal ); - } - - DWORD ret = WaitForSingleObject( hObjectToWaitOn, iMilliseconds ); - switch( ret ) - { - case WAIT_OBJECT_0: - return true; - - case WAIT_ABANDONED: - // The docs aren't particular about what this does, but it should never happen. - FAIL_M( "WAIT_ABANDONED" ); - - case WAIT_TIMEOUT: - return false; - - default: - FAIL_M( "unknown" ); - } -} - // Event logic from http://www.cs.wustl.edu/~schmidt/win32-cv-1.html. bool EventImpl_Win32::Wait( RageTimer *pTimeout ) { @@ -328,28 +315,34 @@ bool EventImpl_Win32::Wait( RageTimer *pTimeout ) } // Unlock the mutex and wait for a signal. - bool bSuccess = PortableSignalObjectAndWait( m_pParent->mutex, m_WakeupSema, true, iMilliseconds ); + bool bSuccess = (SignalObjectAndWait(m_pParent->mutex, m_WakeupSema, iMilliseconds, FALSE) == WAIT_OBJECT_0); EnterCriticalSection( &m_iNumWaitingLock ); if( !bSuccess ) { /* Avoid a race condition: someone may have signalled the object - * between PortableSignalObjectAndWait and EnterCriticalSection. + * between SignalObjectAndWait and EnterCriticalSection. * While we hold m_iNumWaitingLock, poll (with a zero timeout) the * object one last time. */ if( WaitForSingleObject( m_WakeupSema, 0 ) == WAIT_OBJECT_0 ) + { bSuccess = true; + } } --m_iNumWaiting; - bool bLastWaiting = m_iNumWaiting == 0; + bool bLastWaiting = (m_iNumWaiting == 0); LeaveCriticalSection( &m_iNumWaitingLock ); /* If we're the last waiter to wake up, and we were actually woken by * another thread (not by timeout), wake up the signaller. */ - if( bLastWaiting && bSuccess ) - PortableSignalObjectAndWait( m_WaitersDone, m_pParent->mutex, false ); + if (bLastWaiting && bSuccess) + { + SignalObjectAndWait(m_WaitersDone, m_pParent->mutex, INFINITE, FALSE); + } else - WaitForSingleObject( m_pParent->mutex, INFINITE ); + { + WaitForSingleObject(m_pParent->mutex, INFINITE); + } return bSuccess; } @@ -393,7 +386,7 @@ void EventImpl_Win32::Broadcast() EventImpl *MakeEvent( MutexImpl *pMutex ) { - MutexImpl_Win32 *pWin32Mutex = (MutexImpl_Win32 *) pMutex; + MutexImpl_Win32* pWin32Mutex = static_cast(pMutex); return new EventImpl_Win32( pWin32Mutex ); }