From 9e7982f8ec91e05dbdb19cd254809cc802f03cd9 Mon Sep 17 00:00:00 2001 From: Povilas Kanapickas Date: Wed, 3 Nov 2021 02:58:23 +0200 Subject: [PATCH] lib: Switch ArchMutexLock to use std::unique_lock behind the scenes --- src/lib/arch/Arch.h | 18 ++++++++++++------ src/lib/arch/IArchMultithread.cpp | 8 +++----- src/lib/arch/IArchMultithread.h | 3 ++- src/lib/arch/win32/ArchDaemonWindows.cpp | 9 ++++++--- src/lib/arch/win32/ArchTaskBarWindows.cpp | 6 ++---- src/lib/base/SimpleEventQueueBuffer.cpp | 2 +- src/lib/ipc/IpcLogOutputter.cpp | 2 +- src/lib/mt/CondVar.cpp | 3 ++- src/test/mock/ipc/MockIpcServer.h | 3 ++- 9 files changed, 31 insertions(+), 23 deletions(-) diff --git a/src/lib/arch/Arch.h b/src/lib/arch/Arch.h index 3b37617b..17088ab6 100644 --- a/src/lib/arch/Arch.h +++ b/src/lib/arch/Arch.h @@ -64,6 +64,8 @@ # include "arch/unix/ArchInternetUnix.h" #endif +#include + /*! \def ARCH This macro evaluates to the singleton Arch object. @@ -125,15 +127,19 @@ private: //! Convenience object to lock/unlock an arch mutex class ArchMutexLock { public: - ArchMutexLock(ArchMutex mutex) : m_mutex(mutex) - { - ARCH->lockMutex(m_mutex); - } + ArchMutexLock(ArchMutex mutex) : lock{*mutex} {} + ArchMutexLock(ArchMutex mutex, std::adopt_lock_t) : + lock{*mutex, std::adopt_lock}, adopted_{true} + {} + ~ArchMutexLock() { - ARCH->unlockMutex(m_mutex); + if (adopted_) { + lock.release(); + } } + std::unique_lock lock; private: - ArchMutex m_mutex; + bool adopted_ = false; }; diff --git a/src/lib/arch/IArchMultithread.cpp b/src/lib/arch/IArchMultithread.cpp index e5badfa5..d63f1255 100644 --- a/src/lib/arch/IArchMultithread.cpp +++ b/src/lib/arch/IArchMultithread.cpp @@ -17,12 +17,11 @@ */ #include "IArchMultithread.h" +#include "arch/Arch.h" -bool IArchMultithread::waitCondVar(ArchCond cond, ArchMutex mutex, +bool IArchMultithread::waitCondVar(ArchCond cond, ArchMutexLock& lock, double timeout) { - std::unique_lock lock{*mutex, std::adopt_lock}; - // we can't wait on a condition variable and also wake it up for // cancellation since we don't use posix cancellation. so we // must wake up periodically to check for cancellation. we @@ -39,8 +38,7 @@ bool IArchMultithread::waitCondVar(ArchCond cond, ArchMutex mutex, // see if we should cancel this thread testCancelThread(); - auto ret = cond->wait_for(lock, seconds_to_chrono(timeout)); - lock.release(); + auto ret = cond->wait_for(lock.lock, seconds_to_chrono(timeout)); // check for cancel again testCancelThread(); diff --git a/src/lib/arch/IArchMultithread.h b/src/lib/arch/IArchMultithread.h index 98f7e655..5d49245c 100644 --- a/src/lib/arch/IArchMultithread.h +++ b/src/lib/arch/IArchMultithread.h @@ -32,6 +32,7 @@ using ArchMutex = std::mutex*; An architecture dependent type holding the necessary data for a thread. */ class ArchThreadImpl; +class ArchMutexLock; /*! \var ArchThread @@ -107,7 +108,7 @@ public: (Cancellation point) */ - bool waitCondVar(ArchCond cv, ArchMutex mutex, double timeout); + bool waitCondVar(ArchCond cv, ArchMutexLock& lock, double timeout); // // mutex methods diff --git a/src/lib/arch/win32/ArchDaemonWindows.cpp b/src/lib/arch/win32/ArchDaemonWindows.cpp index df0a4cec..2401088f 100644 --- a/src/lib/arch/win32/ArchDaemonWindows.cpp +++ b/src/lib/arch/win32/ArchDaemonWindows.cpp @@ -334,7 +334,8 @@ ArchDaemonWindows::doRunDaemon(RunFunc run) // wait until we're told to start while (!isRunState(m_serviceState) && m_serviceState != SERVICE_STOP_PENDING) { - ARCH->waitCondVar(m_serviceCondVar, m_serviceMutex, -1.0); + ArchMutexLock lock{m_serviceMutex, std::adopt_lock}; + ARCH->waitCondVar(m_serviceCondVar, lock, -1.0); } // run unless told to stop @@ -581,7 +582,8 @@ ArchDaemonWindows::serviceHandler(DWORD ctrl) setStatus(m_serviceState, 0, 5000); PostThreadMessage(m_daemonThreadID, m_quitMessage, 0, 0); while (isRunState(m_serviceState)) { - ARCH->waitCondVar(m_serviceCondVar, m_serviceMutex, -1.0); + ArchMutexLock lock{m_serviceMutex, std::adopt_lock}; + ARCH->waitCondVar(m_serviceCondVar, lock, -1.0); } break; @@ -599,7 +601,8 @@ ArchDaemonWindows::serviceHandler(DWORD ctrl) PostThreadMessage(m_daemonThreadID, m_quitMessage, 0, 0); ARCH->broadcastCondVar(m_serviceCondVar); while (isRunState(m_serviceState)) { - ARCH->waitCondVar(m_serviceCondVar, m_serviceMutex, -1.0); + ArchMutexLock lock{m_serviceMutex, std::adopt_lock}; + ARCH->waitCondVar(m_serviceCondVar, lock, -1.0); } break; diff --git a/src/lib/arch/win32/ArchTaskBarWindows.cpp b/src/lib/arch/win32/ArchTaskBarWindows.cpp index bf71b741..a6814d14 100644 --- a/src/lib/arch/win32/ArchTaskBarWindows.cpp +++ b/src/lib/arch/win32/ArchTaskBarWindows.cpp @@ -81,7 +81,7 @@ ArchTaskBarWindows::init() // we're going to want to get a result from the thread we're // about to create to know if it initialized successfully. // so we lock the condition variable. - ARCH->lockMutex(m_mutex); + ArchMutexLock lock(m_mutex); // open a window and run an event loop in a separate thread. // this has to happen in a separate thread because if we @@ -92,11 +92,9 @@ ArchTaskBarWindows::init() // wait for child thread while (!m_ready) { - ARCH->waitCondVar(m_condVar, m_mutex, -1.0); + ARCH->waitCondVar(m_condVar, lock, -1.0); } - // ready - ARCH->unlockMutex(m_mutex); } void diff --git a/src/lib/base/SimpleEventQueueBuffer.cpp b/src/lib/base/SimpleEventQueueBuffer.cpp index a26f2550..93144bb8 100644 --- a/src/lib/base/SimpleEventQueueBuffer.cpp +++ b/src/lib/base/SimpleEventQueueBuffer.cpp @@ -52,7 +52,7 @@ SimpleEventQueueBuffer::waitForEvent(double timeout) return; } } - ARCH->waitCondVar(m_queueReadyCond, m_queueMutex, timeLeft); + ARCH->waitCondVar(m_queueReadyCond, lock, timeLeft); } } diff --git a/src/lib/ipc/IpcLogOutputter.cpp b/src/lib/ipc/IpcLogOutputter.cpp index 5118a0a6..72bd5c8d 100644 --- a/src/lib/ipc/IpcLogOutputter.cpp +++ b/src/lib/ipc/IpcLogOutputter.cpp @@ -149,7 +149,7 @@ void IpcLogOutputter::buffer_thread() while (isRunning()) { if (m_buffer.empty() || !m_ipcServer.hasClients(m_clientType)) { ArchMutexLock lock(m_notifyMutex); - ARCH->waitCondVar(m_notifyCond, m_notifyMutex, -1); + ARCH->waitCondVar(m_notifyCond, lock, -1); } sendBuffer(); diff --git a/src/lib/mt/CondVar.cpp b/src/lib/mt/CondVar.cpp index b9cf5654..89d7ba40 100644 --- a/src/lib/mt/CondVar.cpp +++ b/src/lib/mt/CondVar.cpp @@ -81,7 +81,8 @@ CondVarBase::wait(Stopwatch& timer, double timeout) const bool CondVarBase::wait(double timeout) const { - return ARCH->waitCondVar(m_cond, m_mutex->m_mutex, timeout); + ArchMutexLock lock{m_mutex->m_mutex, std::adopt_lock}; + return ARCH->waitCondVar(m_cond, lock, timeout); } Mutex* diff --git a/src/test/mock/ipc/MockIpcServer.h b/src/test/mock/ipc/MockIpcServer.h index 5b09b399..1d986458 100644 --- a/src/test/mock/ipc/MockIpcServer.h +++ b/src/test/mock/ipc/MockIpcServer.h @@ -54,7 +54,8 @@ public: } void waitForSend() { - ARCH->waitCondVar(m_sendCond, m_sendMutex, 5); + ArchMutexLock lock{m_sendMutex, std::adopt_lock}; + ARCH->waitCondVar(m_sendCond, lock, 5); } private: