From fe7fd6b0b3221fb14f3457b5332c63d48d7ac7be Mon Sep 17 00:00:00 2001 From: dean11 Date: Wed, 27 Nov 2013 21:47:32 +0100 Subject: [PATCH] Fixed bugs in the threading wrapper --- Code/Misc/Thread/OysterMutex.cpp | 5 - Code/Misc/Thread/OysterMutex.h | 2 - Code/Misc/Thread/OysterThread.h | 6 +- Code/Misc/Thread/OysterThread_Impl.cpp | 124 +++++++++++++++---------- Code/Misc/Utilities-InlineImpl.h | 17 ++-- Code/Misc/Utilities.h | 1 + 6 files changed, 86 insertions(+), 69 deletions(-) diff --git a/Code/Misc/Thread/OysterMutex.cpp b/Code/Misc/Thread/OysterMutex.cpp index a8dfd20f..089323ce 100644 --- a/Code/Misc/Thread/OysterMutex.cpp +++ b/Code/Misc/Thread/OysterMutex.cpp @@ -63,9 +63,4 @@ void OysterMutex::UnlockMutex() bool OysterMutex::IsTaken() { return !this->mutex.try_lock(); -} -void OysterMutex::Reset() -{ - if(!this->mutex.try_lock()) - this->mutex.unlock(); } \ No newline at end of file diff --git a/Code/Misc/Thread/OysterMutex.h b/Code/Misc/Thread/OysterMutex.h index b36585c1..18282499 100644 --- a/Code/Misc/Thread/OysterMutex.h +++ b/Code/Misc/Thread/OysterMutex.h @@ -20,8 +20,6 @@ public: void UnlockMutex(); /** Returns true if mutex is taken */ bool IsTaken(); - /** This function resets resource locking */ - void Reset(); private: std::mutex mutex; diff --git a/Code/Misc/Thread/OysterThread.h b/Code/Misc/Thread/OysterThread.h index 873497ad..05a9f8ad 100644 --- a/Code/Misc/Thread/OysterThread.h +++ b/Code/Misc/Thread/OysterThread.h @@ -6,7 +6,6 @@ #define MISC_OYSTER_THREAD_H #include "IThreadObject.h" - namespace Oyster { namespace Thread @@ -23,11 +22,10 @@ namespace Oyster struct PrivateData; PrivateData *privateData; - OysterThread(const OysterThread& original); - const OysterThread& operator=(const OysterThread& original); - public: OysterThread(); + OysterThread(const OysterThread& original); + const OysterThread& operator=(const OysterThread& original); virtual~OysterThread(); OYSTER_THREAD_ERROR Create(IThreadObject* worker, bool start); diff --git a/Code/Misc/Thread/OysterThread_Impl.cpp b/Code/Misc/Thread/OysterThread_Impl.cpp index 171c8aa9..9b01e6d5 100644 --- a/Code/Misc/Thread/OysterThread_Impl.cpp +++ b/Code/Misc/Thread/OysterThread_Impl.cpp @@ -7,6 +7,7 @@ #include "..\Utilities.h" #include #include +#include using namespace Oyster::Thread; using namespace Utility::DynamicMemory::SmartPointer; @@ -24,24 +25,23 @@ using namespace Utility::DynamicMemory::SmartPointer; OYSTER_THREAD_STATE_RUNNING, OYSTER_THREAD_STATE_PAUSED, OYSTER_THREAD_STATE_STOPED, - OYSTER_THREAD_STATE_TERMINATED, OYSTER_THREAD_STATE_DEAD, }; - //TODO: Add a threadStartPackage struct that contains all the necasary data to fire of a thread + struct ThreadData { - OYSTER_THREAD_STATE state; // workerThread; // state; // workerThread; // msec; //callingThread; threadData->state = OYSTER_THREAD_STATE_STOPED; } + PrivateData(const PrivateData& o) + { + threadData = o.threadData; + } ~PrivateData() { //@todo TODO: Make detatch avalible. @@ -70,35 +74,46 @@ using namespace Utility::DynamicMemory::SmartPointer; #pragma endregion - +int tempId = 0; +std::vector IDS; static void ThreadingFunction(StdSmartPointer &origin) { + bool shouldContinue; StdSmartPointer w = origin; theBegining: - while(w->state == OYSTER_THREAD_STATE_STOPED); - w->mutexLock.LockMutex(); - w->owner->ThreadEntry(); - w->mutexLock.UnlockMutex(); + while(w->state == OYSTER_THREAD_STATE_STOPED) + { + std::this_thread::yield(); + } + +// w->mutexLock.LockMutex(); + if(w->owner) + { + w->owner->ThreadEntry(); + } +// w->mutexLock.UnlockMutex(); while (w->state != OYSTER_THREAD_STATE_STOPED && w->state != OYSTER_THREAD_STATE_DEAD) { - - w->mutexLock.LockMutex(); +// w->mutexLock.LockMutex(); { - shouldContinue = w->owner->DoWork(); + if(w->owner) + { + shouldContinue = w->owner->DoWork(); + } } - w->mutexLock.UnlockMutex(); +// w->mutexLock.UnlockMutex(); if(!shouldContinue) { goto theEnd; } - if(w->state == OYSTER_THREAD_STATE_TERMINATED) + if(w->state == OYSTER_THREAD_STATE_DEAD) { - return; + goto theEnd; } else if(w->state == OYSTER_THREAD_STATE_RESET) { @@ -109,19 +124,26 @@ theBegining: std::this_thread::sleep_for(std::chrono::milliseconds(w->msec)); } - while (w->state == OYSTER_THREAD_STATE_PAUSED); + while (w->state == OYSTER_THREAD_STATE_PAUSED) + { + std::this_thread::yield(); + } } if(w->state == OYSTER_THREAD_STATE_DEAD) { + w->workerThread->detach(); return; } theEnd: - w->mutexLock.LockMutex(); - w->owner->ThreadExit(); - w->mutexLock.UnlockMutex(); +// w->mutexLock.LockMutex(); + if(w->owner) + { + w->owner->ThreadExit(); + } +// w->mutexLock.UnlockMutex(); w->state = OYSTER_THREAD_STATE_DEAD; } @@ -130,6 +152,14 @@ OysterThread::OysterThread() { this->privateData = new PrivateData(); } +OysterThread::OysterThread(const OysterThread& original) +{ + this->privateData = new PrivateData(*original.privateData); +} +const OysterThread& OysterThread::operator=(const OysterThread& original) +{ + return *this; +} OysterThread::~OysterThread() { delete this->privateData; @@ -153,30 +183,33 @@ OYSTER_THREAD_ERROR OysterThread::Create(IThreadObject* worker, bool start) if(start) { - //@todo TODO: No need to lock since the other thread end is only reading this value. Worst case scenario is n lost cycles. this->privateData->threadData->state = OYSTER_THREAD_STATE_RUNNING; } return OYSTER_THREAD_ERROR_SUCCESS; } OYSTER_THREAD_ERROR OysterThread::Start() { + if(!this->privateData->threadData->owner) + return OYSTER_THREAD_ERROR_FAILED; if(!this->privateData->threadData->workerThread) return OYSTER_THREAD_ERROR_FAILED; + if(this->privateData->threadData->state == OYSTER_THREAD_STATE_DEAD) + return OYSTER_THREAD_ERROR_FAILED; this->privateData->threadData->state = OYSTER_THREAD_STATE_RUNNING; return OYSTER_THREAD_ERROR_SUCCESS; } void OysterThread::Stop() { - this->privateData->threadData->mutexLock.LockMutex(); - this->privateData->threadData->state = OYSTER_THREAD_STATE_STOPED; - this->privateData->threadData->mutexLock.UnlockMutex(); + //this->privateData->threadData->mutexLock.LockMutex(); + this->privateData->threadData->state = OYSTER_THREAD_STATE_STOPED; + //this->privateData->threadData->mutexLock.UnlockMutex(); } void OysterThread::Pause() { - this->privateData->threadData->mutexLock.LockMutex(); - this->privateData->threadData->state = OYSTER_THREAD_STATE_PAUSED; - this->privateData->threadData->mutexLock.UnlockMutex(); + //this->privateData->threadData->mutexLock.LockMutex(); + this->privateData->threadData->state = OYSTER_THREAD_STATE_PAUSED; + //this->privateData->threadData->mutexLock.UnlockMutex(); } void OysterThread::Pause(int msec) { @@ -187,39 +220,34 @@ void OysterThread::Pause(int msec) } else { - this->privateData->threadData->mutexLock.LockMutex(); - this->privateData->threadData->state = OYSTER_THREAD_STATE_PAUSED; - this->privateData->threadData->msec = msec; - this->privateData->threadData->mutexLock.UnlockMutex(); + //this->privateData->threadData->mutexLock.LockMutex(); + this->privateData->threadData->state = OYSTER_THREAD_STATE_PAUSED; + this->privateData->threadData->msec = msec; + //this->privateData->threadData->mutexLock.UnlockMutex(); } } void OysterThread::Resume() { - this->privateData->threadData->mutexLock.LockMutex(); +// this->privateData->threadData->mutexLock.LockMutex(); this->privateData->threadData->state = OYSTER_THREAD_STATE_RUNNING; - this->privateData->threadData->mutexLock.UnlockMutex(); +// this->privateData->threadData->mutexLock.UnlockMutex(); } OYSTER_THREAD_ERROR OysterThread::Reset(IThreadObject* worker) { - this->privateData->threadData->mutexLock.LockMutex(); +// this->privateData->threadData->mutexLock.LockMutex(); if(worker) { this->privateData->threadData->owner = worker; } this->privateData->threadData->callingThread = std::this_thread::get_id(); this->privateData->threadData->msec = 0; - this->privateData->threadData->mutexLock.UnlockMutex(); +// this->privateData->threadData->mutexLock.UnlockMutex(); return OYSTER_THREAD_ERROR_SUCCESS; } void OysterThread::Terminate() { - delete this->privateData->threadData->workerThread; - this->privateData->threadData->mutexLock.Reset(); - this->privateData->threadData->workerThread = 0; - this->privateData->threadData->callingThread = std::thread::id(); - this->privateData->threadData->msec = 0; - this->privateData->threadData->state = OYSTER_THREAD_STATE_STOPED; + this->privateData->threadData->state = OYSTER_THREAD_STATE_DEAD; } void OysterThread::Wait() { @@ -250,4 +278,4 @@ bool OysterThread::IsActive() return false; } - \ No newline at end of file + diff --git a/Code/Misc/Utilities-InlineImpl.h b/Code/Misc/Utilities-InlineImpl.h index b8c4c6da..0cd164d8 100644 --- a/Code/Misc/Utilities-InlineImpl.h +++ b/Code/Misc/Utilities-InlineImpl.h @@ -165,8 +165,7 @@ namespace Utility namespace SmartPointer { - template - void StdSmartPointer::Destroy() + template void StdSmartPointer::Destroy() { delete this->_rc; this->_rc = NULL; @@ -200,7 +199,7 @@ namespace Utility if (this != &p) { //Last to go? - if(this->_rc && this->_rc->Release() == 0) + if(this->_rc && this->_rc->Decref() == 0) { //Call child specific Destroy(); @@ -208,7 +207,7 @@ namespace Utility this->_ptr = p._ptr; this->_rc = p._rc; - this->_rc->Add(); + this->_rc->Incref(); } return *this; } @@ -254,16 +253,14 @@ namespace Utility { return this->_ptr; } - - /** - * Returns the connected pointer */ + template inline StdSmartPointer::operator bool() + { + return (this->_ptr != 0); + } template inline T* StdSmartPointer::Get() { return this->_ptr; } - - /** Checks if the pointer is valid (not NULL) - Returns true for valid, else false. */ template inline bool StdSmartPointer::IsValid() { return (this->_ptr != NULL) ? true : false; diff --git a/Code/Misc/Utilities.h b/Code/Misc/Utilities.h index e1582697..8eaf18f4 100644 --- a/Code/Misc/Utilities.h +++ b/Code/Misc/Utilities.h @@ -151,6 +151,7 @@ namespace Utility T& operator* (); T* operator-> (); operator T* (); + operator bool(); /** * Returns the connected pointer */