From a1d6fa071280123f282c1bac3b68008672401024 Mon Sep 17 00:00:00 2001 From: Mikko Rasa Date: Sat, 28 May 2011 13:05:30 +0300 Subject: [PATCH] Refactor Mutex with pimpl Rewrite the win32 implementation to use a critical section Change the logic of Semaphore, as the win32 condvar-emulation was rather dodgy --- source/core/mutex.cpp | 62 ++++++++++++++++ source/core/mutex.h | 37 +++++----- source/core/mutex_private.h | 24 +++++++ source/core/semaphore.cpp | 138 ++++++++++++++++-------------------- source/core/semaphore.h | 35 ++------- 5 files changed, 176 insertions(+), 120 deletions(-) create mode 100644 source/core/mutex.cpp create mode 100644 source/core/mutex_private.h diff --git a/source/core/mutex.cpp b/source/core/mutex.cpp new file mode 100644 index 0000000..e1657c9 --- /dev/null +++ b/source/core/mutex.cpp @@ -0,0 +1,62 @@ +#ifndef WIN32 +#include +#endif +#include "mutex.h" +#include "mutex_private.h" +#include "systemerror.h" + +namespace Msp { + +Mutex::Mutex(): + priv(new Private) +{ +#ifdef WIN32 + InitializeCriticalSection(&priv->crit); +#else + pthread_mutex_init(&priv->mutex, 0); +#endif +} + +Mutex::~Mutex() +{ +#ifdef WIN32 + DeleteCriticalSection(&priv->crit); +#else + pthread_mutex_destroy(&priv->mutex); +#endif + delete priv; +} + +void Mutex::lock() +{ +#ifdef WIN32 + EnterCriticalSection(&priv->crit); +#else + if(int err = pthread_mutex_lock(&priv->mutex)) + throw system_error("Mutex::lock", err); +#endif +} + +bool Mutex::trylock() +{ +#ifdef WIN32 + return TryEnterCriticalSection(&priv->crit); +#else + int err = pthread_mutex_trylock(&priv->mutex); + if(err && err!=EBUSY) + throw system_error("Mutex::trylock", err); + return !err; +#endif +} + +void Mutex::unlock() +{ +#ifdef WIN32 + LeaveCriticalSection(&priv->crit); +#else + if(int err = pthread_mutex_unlock(&priv->mutex)) + throw system_error("Mutex::unlock", err); +#endif +} + +} // namespace Msp diff --git a/source/core/mutex.h b/source/core/mutex.h index c0c2682..b739134 100644 --- a/source/core/mutex.h +++ b/source/core/mutex.h @@ -9,31 +9,36 @@ Distributed under the LGPL #define MSP_CORE_MUTEX_H_ #include "refptr.h" -#include "types.h" namespace Msp { +/** +A class for controlling mutually exclusive access to a resource. Only one +thread can hold a lock on the mutex at a time. +*/ class Mutex { friend class Semaphore; private: - MutexHandle mutex; + struct Private; + + Private *priv; public: -#ifndef WIN32 - Mutex() { pthread_mutex_init(&mutex, 0); } - int lock() { return pthread_mutex_lock(&mutex); } - int trylock() { return pthread_mutex_trylock(&mutex); } - int unlock() { return pthread_mutex_unlock(&mutex); } - ~Mutex() { pthread_mutex_destroy(&mutex); } -#else - Mutex() { mutex = CreateMutex(0, false, 0); } - int lock() { return WaitForSingleObject(mutex, INFINITE)==WAIT_OBJECT_0; } - int trylock() { return WaitForSingleObject(mutex, 0)==WAIT_OBJECT_0; } - int unlock() { return !ReleaseMutex(mutex); } - ~Mutex() { CloseHandle(mutex); } -#endif + Mutex(); + ~Mutex(); + + /** Locks the mutex. If the mutex is already locked, waits until it becomes + available. */ + void lock(); + + /** Attempts to lock the mutex. Returns true if the lock was acquired, + otherwise returns false. */ + bool trylock(); + + /** Unlocks the mutex. */ + void unlock(); }; /** @@ -48,7 +53,7 @@ public: MutexLock(Mutex &m, bool l = true): mutex(m) { if(l) mutex.lock(); } ~MutexLock() { mutex.unlock(); } - int lock() { return mutex.lock(); } + void lock() { mutex.lock(); } private: MutexLock(const MutexLock &); MutexLock &operator=(const MutexLock &); diff --git a/source/core/mutex_private.h b/source/core/mutex_private.h new file mode 100644 index 0000000..0b092da --- /dev/null +++ b/source/core/mutex_private.h @@ -0,0 +1,24 @@ +#ifndef MSP_CORE_MUTEX_PRIVATE_H_ +#define MSP_CORE_MUTEX_PRIVATE_H_ + +#ifdef WIN32 +#include +#else +#include +#endif +#include "mutex.h" + +namespace Msp { + +struct Mutex::Private +{ +#ifdef WIN32 + CRITICAL_SECTION crit; +#else + pthread_mutex_t mutex; +#endif +}; + +} // namespace Msp + +#endif diff --git a/source/core/semaphore.cpp b/source/core/semaphore.cpp index 139cae2..544fb27 100644 --- a/source/core/semaphore.cpp +++ b/source/core/semaphore.cpp @@ -5,117 +5,103 @@ Copyright © 2006 Mikko Rasa, Mikkosoft Productions Distributed under the LGPL */ -#ifndef WIN32 +#ifdef WIN32 +#include +#else #include +#include #endif -#include +#include +#include +#include +#include "mutex_private.h" #include "semaphore.h" -#include "../time/timestamp.h" -#include "../time/units.h" -#include "../time/utils.h" +#include "systemerror.h" namespace Msp { -Semaphore::Semaphore(): - mutex(new Mutex), - own_mutex(true) +struct Semaphore::Private { - init(); -} +#ifdef WIN32 + HANDLE handle; +#else + Mutex mutex; + pthread_cond_t cond; + unsigned limit; + unsigned count; +#endif +}; -Semaphore::Semaphore(Mutex &m): - mutex(&m), - own_mutex(false) -{ - init(); -} -void Semaphore::init() +Semaphore::Semaphore(unsigned limit): + priv(new Private) { #ifdef WIN32 - count = 0; - sem = CreateSemaphore(0, 0, 32, 0); + priv->handle = CreateSemaphore(0, 0, limit, 0); #else - pthread_cond_init(&sem, 0); + pthread_cond_init(&priv->cond, 0); + priv->limit = limit; + priv->count = 0; #endif } Semaphore::~Semaphore() { - if(own_mutex) - delete mutex; #ifdef WIN32 - CloseHandle(sem); + CloseHandle(priv->handle); #else - pthread_cond_destroy(&sem); + pthread_cond_destroy(&priv->cond); #endif + delete priv; } -#ifdef WIN32 -int Semaphore::signal() -{ - if(count==0) - return 0; - - int ret = !ReleaseSemaphore(sem, 1, 0); - - unsigned old_count = count; - mutex->unlock(); - while(count==old_count) - Sleep(0); - mutex->lock(); - - return ret; -} - -int Semaphore::broadcast() +void Semaphore::signal() { - if(count==0) - return 0; - int ret = !ReleaseSemaphore(sem, count, 0); - - mutex->unlock(); - while(count) - Sleep(0); - mutex->lock(); - - return ret; +#ifdef WIN32 + if(!ReleaseSemaphore(priv->handle, 1, 0)) + throw system_error("Semaphore::signal"); +#else + MutexLock mlock(priv->mutex); + if(priv->countlimit) + ++priv->count; + if(int err = pthread_cond_signal(&priv->cond)) + throw system_error("Semaphore::signal", err); +#endif } -int Semaphore::wait() +void Semaphore::wait() { - ++count; - mutex->unlock(); - DWORD ret = WaitForSingleObject(sem, INFINITE); - mutex->lock(); - --count; - - return ret==WAIT_OBJECT_0; -} +#ifdef WIN32 + DWORD ret = WaitForSingleObject(priv->handle, INFINITE); + if(ret==WAIT_FAILED) + throw system_error("Semaphore::wait"); +#else + MutexLock mlock(priv->mutex); + while(!priv->count) + if(int err = pthread_cond_wait(&priv->cond, &priv->mutex.priv->mutex)) + throw system_error("Semaphore::wait", err); + --priv->count; #endif +} -int Semaphore::wait(const Time::TimeDelta &d) +bool Semaphore::wait(const Time::TimeDelta &d) { -#ifndef WIN32 +#ifdef WIN32 + DWORD ret = WaitForSingleObject(priv->handle, (DWORD)(d/Time::usec)); + if(ret==WAIT_FAILED) + throw system_error("Semaphore::wait"); + return ret==WAIT_OBJECT_0; +#else Time::TimeStamp ts = Time::now()+d; timespec timeout; timeout.tv_sec = ts.raw()/1000000; timeout.tv_nsec = (ts.raw()%1000000)*1000; - int r = pthread_cond_timedwait(&sem, &mutex->mutex, &timeout); - if(r==ETIMEDOUT) - return 1; - else if(r) - return -1; - return 0; -#else - ++count; - mutex->lock(); - DWORD ret = WaitForSingleObject(sem, (DWORD)(d/Time::usec)); - mutex->unlock(); - --count; - return ret==WAIT_OBJECT_0; + int err = pthread_cond_timedwait(&priv->cond, &priv->mutex.priv->mutex, &timeout); + if(err && err!=ETIMEDOUT) + throw system_error("Semaphore::wait", err); + return err==0; #endif } diff --git a/source/core/semaphore.h b/source/core/semaphore.h index e6c09c5..3f2cc97 100644 --- a/source/core/semaphore.h +++ b/source/core/semaphore.h @@ -9,7 +9,6 @@ Distributed under the LGPL #define MSP_CORE_SEMAPHORE_H_ #include "mutex.h" -#include "types.h" #include "../time/timedelta.h" namespace Msp { @@ -17,39 +16,19 @@ namespace Msp { class Semaphore { private: - Mutex *mutex; - bool own_mutex; - SemaphoreHandle sem; -#ifdef WIN32 - unsigned count; -#endif + struct Private; + + Private *priv; public: - Semaphore(); - Semaphore(Mutex &); -private: - void init(); -public: + Semaphore(unsigned); ~Semaphore(); - int signal(); - int broadcast(); - int wait(); - int wait(const Time::TimeDelta &); - Mutex &get_mutex() { return *mutex; } + void signal(); + void wait(); + bool wait(const Time::TimeDelta &); }; -#ifndef WIN32 -inline int Semaphore::signal() -{ return pthread_cond_signal(&sem); } - -inline int Semaphore::broadcast() -{ return pthread_cond_broadcast(&sem); } - -inline int Semaphore::wait() -{ return pthread_cond_wait(&sem, &mutex->mutex); } -#endif - } #endif -- 2.43.0