From 7ac18c0f22bcd62feede693be2617264eb5decf6 Mon Sep 17 00:00:00 2001 From: frosch Date: Sun, 16 Feb 2014 21:37:05 +0000 Subject: [PATCH] (svn r26349) -Add: Optional recursive locking of mutexes. --- src/thread/thread.h | 10 ++++++++-- src/thread/thread_none.cpp | 4 ++-- src/thread/thread_os2.cpp | 16 +++++++++++++--- src/thread/thread_pthread.cpp | 35 ++++++++++++++++++++++++++++++----- src/thread/thread_win32.cpp | 16 +++++++++++++--- 5 files changed, 66 insertions(+), 15 deletions(-) diff --git a/src/thread/thread.h b/src/thread/thread.h index a98dfb2239..b944a53b96 100644 --- a/src/thread/thread.h +++ b/src/thread/thread.h @@ -66,13 +66,19 @@ public: /** * Begin the critical section + * @param allow_recursive Whether recursive locking is intentional. + * If false, NOT_REACHED() will be called when the mutex is already locked + * by the current thread. */ - virtual void BeginCritical() = 0; + virtual void BeginCritical(bool allow_recursive = false) = 0; /** * End of the critical section + * @param allow_recursive Whether recursive unlocking is intentional. + * If false, NOT_REACHED() will be called when the mutex was locked more + * than once by the current thread. */ - virtual void EndCritical() = 0; + virtual void EndCritical(bool allow_recursive = false) = 0; /** * Wait for a signal to be send. diff --git a/src/thread/thread_none.cpp b/src/thread/thread_none.cpp index 67679bc819..415a62d59d 100644 --- a/src/thread/thread_none.cpp +++ b/src/thread/thread_none.cpp @@ -21,8 +21,8 @@ /** Mutex that doesn't do locking because it ain't needed when there're no threads */ class ThreadMutex_None : public ThreadMutex { public: - virtual void BeginCritical() {} - virtual void EndCritical() {} + virtual void BeginCritical(bool allow_recursive = false) {} + virtual void EndCritical(bool allow_recursive = false) {} virtual void WaitForSignal() {} virtual void SendSignal() {} }; diff --git a/src/thread/thread_os2.cpp b/src/thread/thread_os2.cpp index 903ea0ebe3..56f86275d1 100644 --- a/src/thread/thread_os2.cpp +++ b/src/thread/thread_os2.cpp @@ -95,9 +95,10 @@ class ThreadMutex_OS2 : public ThreadMutex { private: HMTX mutex; ///< The mutex. HEV event; ///< Event for waiting. + uint recursive_count; ///< Recursive lock count. public: - ThreadMutex_OS2() + ThreadMutex_OS2() : recursive_count(0) { DosCreateMutexSem(NULL, &mutex, 0, FALSE); DosCreateEventSem(NULL, &event, 0, FALSE); @@ -109,21 +110,30 @@ public: DosCloseEventSem(event); } - /* virtual */ void BeginCritical() + /* virtual */ void BeginCritical(bool allow_recursive = false) { + /* os2 mutex is recursive by itself */ DosRequestMutexSem(mutex, (unsigned long) SEM_INDEFINITE_WAIT); + this->recursive_count++; + if (!allow_recursive && this->recursive_count != 1) NOT_REACHED(); } - /* virtual */ void EndCritical() + /* virtual */ void EndCritical(bool allow_recursive = false) { + if (!allow_recursive && this->recursive_count != 1) NOT_REACHED(); + this->recursive_count--; DosReleaseMutexSem(mutex); } /* virtual */ void WaitForSignal() { + assert(this->recursive_count == 1); // Do we need to call Begin/EndCritical multiple times otherwise? + uint old_recursive_count = this->recursive_count; + this->recursive_count = 0; this->EndCritical(); DosWaitEventSem(event, SEM_INDEFINITE_WAIT); this->BeginCritical(); + this->recursive_count = this->recursive_count; } /* virtual */ void SendSignal() diff --git a/src/thread/thread_pthread.cpp b/src/thread/thread_pthread.cpp index 11cd3accd7..9d54a4d20b 100644 --- a/src/thread/thread_pthread.cpp +++ b/src/thread/thread_pthread.cpp @@ -98,9 +98,11 @@ private: pthread_mutex_t mutex; ///< The actual mutex. pthread_cond_t condition; ///< Data for conditional waiting. pthread_mutexattr_t attr; ///< Attributes set for the mutex. + pthread_t owner; ///< Owning thread of the mutex. + uint recursive_count; ///< Recursive lock count. public: - ThreadMutex_pthread() + ThreadMutex_pthread() : owner(0), recursive_count(0) { pthread_mutexattr_init(&this->attr); pthread_mutexattr_settype(&this->attr, PTHREAD_MUTEX_ERRORCHECK); @@ -116,22 +118,45 @@ public: assert(err != EBUSY); } - /* virtual */ void BeginCritical() + bool IsOwnedByCurrentThread() const { - int err = pthread_mutex_lock(&this->mutex); - assert(err == 0); + return this->owner == pthread_self(); } - /* virtual */ void EndCritical() + /* virtual */ void BeginCritical(bool allow_recursive = false) { + /* pthread mutex is not recursive by itself */ + if (this->IsOwnedByCurrentThread()) { + if (!allow_recursive) NOT_REACHED(); + } else { + int err = pthread_mutex_lock(&this->mutex); + assert(err == 0); + assert(this->recursive_count == 0); + this->owner = pthread_self(); + } + this->recursive_count++; + } + + /* virtual */ void EndCritical(bool allow_recursive = false) + { + assert(this->IsOwnedByCurrentThread()); + if (!allow_recursive && this->recursive_count != 1) NOT_REACHED(); + this->recursive_count--; + if (this->recursive_count != 0) return; + this->owner = 0; int err = pthread_mutex_unlock(&this->mutex); assert(err == 0); } /* virtual */ void WaitForSignal() { + uint old_recursive_count = this->recursive_count; + this->recursive_count = 0; + this->owner = 0; int err = pthread_cond_wait(&this->condition, &this->mutex); assert(err == 0); + this->owner = pthread_self(); + this->recursive_count = old_recursive_count; } /* virtual */ void SendSignal() diff --git a/src/thread/thread_win32.cpp b/src/thread/thread_win32.cpp index 1e7d0731e8..d002ddb88d 100644 --- a/src/thread/thread_win32.cpp +++ b/src/thread/thread_win32.cpp @@ -108,9 +108,10 @@ class ThreadMutex_Win32 : public ThreadMutex { private: CRITICAL_SECTION critical_section; ///< The critical section we would enter. HANDLE event; ///< Event for signalling. + uint recursive_count; ///< Recursive lock count. public: - ThreadMutex_Win32() + ThreadMutex_Win32() : recursive_count(0) { InitializeCriticalSection(&this->critical_section); this->event = CreateEvent(NULL, FALSE, FALSE, NULL); @@ -122,21 +123,30 @@ public: CloseHandle(this->event); } - /* virtual */ void BeginCritical() + /* virtual */ void BeginCritical(bool allow_recursive = false) { + /* windows mutex is recursive by itself */ EnterCriticalSection(&this->critical_section); + this->recursive_count++; + if (!allow_recursive && this->recursive_count != 1) NOT_REACHED(); } - /* virtual */ void EndCritical() + /* virtual */ void EndCritical(bool allow_recursive = false) { + if (!allow_recursive && this->recursive_count != 1) NOT_REACHED(); + this->recursive_count--; LeaveCriticalSection(&this->critical_section); } /* virtual */ void WaitForSignal() { + assert(this->recursive_count == 1); // Do we need to call Begin/EndCritical multiple times otherwise? + uint old_recursive_count = this->recursive_count; + this->recursive_count = 0; this->EndCritical(); WaitForSingleObject(this->event, INFINITE); this->BeginCritical(); + this->recursive_count = this->recursive_count; } /* virtual */ void SendSignal()