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

diff --git a/source/core/mutex.cpp b/source/core/mutex.cpp
new file mode 100644 (file)
index 0000000..e1657c9
--- /dev/null
+++ b/source/core/mutex.cpp
@@ -0,0 +1,62 @@
+#ifndef WIN32
+#include <cerrno>
+#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 c0c26829630b9dee55bdabf885fd76a6a87ca4f5..b73913450bc2ce5e150047254af6e83a2fbb0839 100644 (file)
--- 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 (file)
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 <windows.h>
+#else
+#include <pthread.h>
+#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 139cae2f4ca523ffac47af9487663799a65c17ea..544fb27f81b192731f4175b97257f7b6e534268c 100644 (file)
--- 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 <windows.h>
+#else
 #include <sys/time.h>
+#include <cerrno>
 #endif
-#include <errno.h>
+#include <msp/time/timestamp.h>
+#include <msp/time/units.h>
+#include <msp/time/utils.h>
+#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->count<priv->limit)
+               ++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 e6c09c54437e57ed3ae0cf8903a30c920c976760..3f2cc974a455855c35246f7abc10d12d1993e709 100644 (file)
--- 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