From 14d2085e35bbc3e5c73c018e5c70e8093003820f Mon Sep 17 00:00:00 2001 From: Tycho Bickerstaff Date: Sat, 21 Dec 2013 14:43:32 +0000 Subject: basic threadsafe queue interface --- src/OSSupport/Queue.h | 30 ++++++++++++++++++++++++++++++ src/OSSupport/Queue.inc | 4 ++++ 2 files changed, 34 insertions(+) create mode 100644 src/OSSupport/Queue.h create mode 100644 src/OSSupport/Queue.inc (limited to 'src/OSSupport') diff --git a/src/OSSupport/Queue.h b/src/OSSupport/Queue.h new file mode 100644 index 000000000..838a101e0 --- /dev/null +++ b/src/OSSupport/Queue.h @@ -0,0 +1,30 @@ +#pragma once + +template +class cDeleter +{ + public: + static void Delete(T) {}; +} + +template +class cQueue +{ +public: + cQueue(int warnsize); + cQueue(cQueue& queue); + ~cQueue(); + + void EnqueueItem(T item); + bool TryDequeueItem(T& item); + T DequeueItem(); + void BlockTillEmpty(cEvent CancelationEvent); + void Clear(); + int Size(); + +private: + int warnsize; +} + +//template classes must be implemented in the header +#include "Queue.inc" diff --git a/src/OSSupport/Queue.inc b/src/OSSupport/Queue.inc new file mode 100644 index 000000000..f172e9794 --- /dev/null +++ b/src/OSSupport/Queue.inc @@ -0,0 +1,4 @@ + +#include "Globals.h" // NOTE: MSVC stupidness requires this to be the same across all modules + + -- cgit v1.2.3 From 725b997a2817bd999079e5e6678f5497373cbbac Mon Sep 17 00:00:00 2001 From: madmaxoft Date: Sat, 28 Dec 2013 21:59:50 +0100 Subject: Fixed a (valid) warning in RCONServer. --- src/OSSupport/SocketThreads.h | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src/OSSupport') diff --git a/src/OSSupport/SocketThreads.h b/src/OSSupport/SocketThreads.h index ecbac3aeb..858729c49 100644 --- a/src/OSSupport/SocketThreads.h +++ b/src/OSSupport/SocketThreads.h @@ -61,6 +61,9 @@ public: class cCallback { public: + // Force a virtual destructor in all subclasses: + virtual ~cCallback() {} + /// Called when data is received from the remote party virtual void DataReceived(const char * a_Data, int a_Size) = 0; -- cgit v1.2.3 From f3736b1eb7bf698518cdb853ee29ee96b9c24a52 Mon Sep 17 00:00:00 2001 From: Tycho Bickerstaff Date: Tue, 31 Dec 2013 15:48:57 +0000 Subject: refactored chunk Queue to seperate class --- src/OSSupport/Event.h | 6 +-- src/OSSupport/IsThread.h | 1 - src/OSSupport/Queue.h | 110 +++++++++++++++++++++++++++++++++++++++-------- src/OSSupport/Queue.inc | 4 -- 4 files changed, 95 insertions(+), 26 deletions(-) delete mode 100644 src/OSSupport/Queue.inc (limited to 'src/OSSupport') diff --git a/src/OSSupport/Event.h b/src/OSSupport/Event.h index 71f418c0c..31f32f8c6 100644 --- a/src/OSSupport/Event.h +++ b/src/OSSupport/Event.h @@ -20,10 +20,10 @@ class cEvent { public: cEvent(void); - ~cEvent(); + virtual ~cEvent(); - void Wait(void); - void Set (void); + virtual void Wait(void); + virtual void Set (void); private: diff --git a/src/OSSupport/IsThread.h b/src/OSSupport/IsThread.h index b8784ea33..af4367636 100644 --- a/src/OSSupport/IsThread.h +++ b/src/OSSupport/IsThread.h @@ -22,7 +22,6 @@ In the descending class' constructor call the Start() method to start the thread - class cIsThread { protected: diff --git a/src/OSSupport/Queue.h b/src/OSSupport/Queue.h index 838a101e0..eb323b067 100644 --- a/src/OSSupport/Queue.h +++ b/src/OSSupport/Queue.h @@ -1,30 +1,104 @@ + #pragma once +#include + +#include "../OSSupport/Promise.h" + +//this empty struct allows function inlining template -class cDeleter +struct cQueueFuncs { public: static void Delete(T) {}; -} + static void Combine(T&, const T) {}; +}; -template +template > class cQueue { + +typedef typename std::list ListType; +//magic typedef to persuade clang that the iterator is a type +typedef typename ListType::iterator iterator; public: - cQueue(int warnsize); - cQueue(cQueue& queue); - ~cQueue(); - - void EnqueueItem(T item); - bool TryDequeueItem(T& item); - T DequeueItem(); - void BlockTillEmpty(cEvent CancelationEvent); - void Clear(); - int Size(); - + cQueue() {} + ~cQueue() {} + + void EnqueueItem(ItemType a_item) + { + cCSLock Lock(m_CS); + m_contents.push_back(a_item); + m_evtAdded.Set(); + } + void EnqueueItemIfNotPresent(ItemType a_item) + { + cCSLock Lock(m_CS); + + for (iterator itr = m_contents.begin(); itr != m_contents.end(); ++itr) + { + if((*itr) == a_item) { + Funcs funcTable; + funcTable.Combine(*itr,a_item); + return; + } + } + m_contents.push_back(a_item); + m_evtAdded.Set(); + } + bool TryDequeueItem(ItemType& item) + { + cCSLock Lock(m_CS); + if (m_contents.size() == 0) return false; + item = m_contents.front(); + m_contents.pop_front(); + return true; + } + ItemType DequeueItem() + { + cCSLock Lock(m_CS); + while (m_contents.size() == 0) + { + cCSUnlock Unlock(m_CS); + m_evtAdded.Wait(); + } + return m_contents.pop_front(); + } + cPromise* BlockTillEmpty() { + return new cEmptyQueuePromise(this); + } + //can all be inlined when delete is a noop + void Clear() + { + cCSLock Lock(m_CS); + Funcs funcTable; + while (!m_contents.empty()) + { + funcTable.Delete(m_contents.front()); + m_contents.pop_front(); + } + } + size_t Size() + { + cCSLock Lock(m_CS); + return m_contents.size(); + } + bool Remove(ItemType item) + { + cCSLock Lock(m_CS); + m_contents.remove(item); + } + private: - int warnsize; -} + ListType m_contents; + cCriticalSection m_CS; + cEvent m_evtAdded; -//template classes must be implemented in the header -#include "Queue.inc" + class cEmptyQueuePromise : public cPromise { + public: + cEmptyQueuePromise(cQueue* a_Queue) : cPromise(), m_Queue(a_Queue) {} + virtual bool IsCompleted() {return m_Queue->Size() != 0;} + private: + cQueue* m_Queue; + }; +}; diff --git a/src/OSSupport/Queue.inc b/src/OSSupport/Queue.inc deleted file mode 100644 index f172e9794..000000000 --- a/src/OSSupport/Queue.inc +++ /dev/null @@ -1,4 +0,0 @@ - -#include "Globals.h" // NOTE: MSVC stupidness requires this to be the same across all modules - - -- cgit v1.2.3 From 512d1b9ebebaf205756dde352c4e714dd632ca2d Mon Sep 17 00:00:00 2001 From: Tycho Bickerstaff Date: Tue, 31 Dec 2013 15:58:21 +0000 Subject: clean up code for patching --- src/OSSupport/Event.h | 6 +++--- src/OSSupport/IsThread.h | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) (limited to 'src/OSSupport') diff --git a/src/OSSupport/Event.h b/src/OSSupport/Event.h index 31f32f8c6..71f418c0c 100644 --- a/src/OSSupport/Event.h +++ b/src/OSSupport/Event.h @@ -20,10 +20,10 @@ class cEvent { public: cEvent(void); - virtual ~cEvent(); + ~cEvent(); - virtual void Wait(void); - virtual void Set (void); + void Wait(void); + void Set (void); private: diff --git a/src/OSSupport/IsThread.h b/src/OSSupport/IsThread.h index af4367636..b8784ea33 100644 --- a/src/OSSupport/IsThread.h +++ b/src/OSSupport/IsThread.h @@ -22,6 +22,7 @@ In the descending class' constructor call the Start() method to start the thread + class cIsThread { protected: -- cgit v1.2.3 From 098ed91a48df9fef58590b22fe34225ed1f44cfa Mon Sep 17 00:00:00 2001 From: Tycho Bickerstaff Date: Tue, 31 Dec 2013 16:13:13 +0000 Subject: fogot to add promise classes --- src/OSSupport/Promise.cpp | 54 +++++++++++++++++++++++++++++++++++++++++++++++ src/OSSupport/Promise.h | 38 +++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+) create mode 100644 src/OSSupport/Promise.cpp create mode 100644 src/OSSupport/Promise.h (limited to 'src/OSSupport') diff --git a/src/OSSupport/Promise.cpp b/src/OSSupport/Promise.cpp new file mode 100644 index 000000000..b31869334 --- /dev/null +++ b/src/OSSupport/Promise.cpp @@ -0,0 +1,54 @@ + +#include "Globals.h" + +#include "Promise.h" + +cPromise * cPromise::WaitFor(cPromise * a_Promise) +{ + return new cCombinedPromise(this, a_Promise); +} + +cPromise * cPromise::CancelOn(volatile bool& cancelation) +{ + return new cCancelablePromise(this, cancelation); +} + +void cPromise::Wait() +{ + while(!IsCompleted()){}; //busywait best we can do until waitany +} + + +cCombinedPromise::cCombinedPromise(cPromise* a_left, cPromise* a_right) : + cPromise(), + m_left(a_left), + m_right(a_right) +{ +} + +cCombinedPromise::~cCombinedPromise() +{ +} + +bool cCombinedPromise::IsCompleted() +{ + return m_left->IsCompleted() || m_right->IsCompleted(); +} + +cCancelablePromise::cCancelablePromise(cPromise* a_wrapped, volatile bool& a_cancel) : + cPromise(), + m_cancel(a_cancel), + m_wrapped(a_wrapped) +{ +} + +cCancelablePromise::~cCancelablePromise () +{ +} + +bool cCancelablePromise::IsCompleted() +{ + return m_cancel || m_wrapped->IsCompleted(); +} + + diff --git a/src/OSSupport/Promise.h b/src/OSSupport/Promise.h new file mode 100644 index 000000000..83d04860b --- /dev/null +++ b/src/OSSupport/Promise.h @@ -0,0 +1,38 @@ +#pragma once + +class cCombinedPromise; + + +class cPromise { + public: + cPromise() {} + virtual ~cPromise () {} + cPromise * WaitFor(cPromise * a_Promise); + cPromise * CancelOn(volatile bool& cancelationtoken); + void Wait(); + virtual bool IsCompleted() = 0; + //TODO:Expose Events for waiting on +}; + +class cCombinedPromise : public cPromise { +public: + cCombinedPromise(cPromise*, cPromise*); + ~cCombinedPromise(); + virtual bool IsCompleted(); +private: + cPromise* m_left; + cPromise* m_right; +}; + +class cCancelablePromise : public cPromise { +public: + cCancelablePromise(cPromise*, volatile bool&); + ~cCancelablePromise(); + virtual bool IsCompleted(); +private: + volatile bool& m_cancel; + cPromise* m_wrapped; +}; + + + -- cgit v1.2.3 From 042b72bc172e7eb4e9ef7668ae28be6e7a3b4036 Mon Sep 17 00:00:00 2001 From: Tycho Bickerstaff Date: Thu, 2 Jan 2014 12:32:55 +0000 Subject: rewrote queue not to use promises for waits --- src/OSSupport/Promise.cpp | 54 ----------------------------------------------- src/OSSupport/Promise.h | 38 --------------------------------- src/OSSupport/Queue.h | 24 ++++++++++----------- 3 files changed, 11 insertions(+), 105 deletions(-) delete mode 100644 src/OSSupport/Promise.cpp delete mode 100644 src/OSSupport/Promise.h (limited to 'src/OSSupport') diff --git a/src/OSSupport/Promise.cpp b/src/OSSupport/Promise.cpp deleted file mode 100644 index b31869334..000000000 --- a/src/OSSupport/Promise.cpp +++ /dev/null @@ -1,54 +0,0 @@ - -#include "Globals.h" - -#include "Promise.h" - -cPromise * cPromise::WaitFor(cPromise * a_Promise) -{ - return new cCombinedPromise(this, a_Promise); -} - -cPromise * cPromise::CancelOn(volatile bool& cancelation) -{ - return new cCancelablePromise(this, cancelation); -} - -void cPromise::Wait() -{ - while(!IsCompleted()){}; //busywait best we can do until waitany -} - - -cCombinedPromise::cCombinedPromise(cPromise* a_left, cPromise* a_right) : - cPromise(), - m_left(a_left), - m_right(a_right) -{ -} - -cCombinedPromise::~cCombinedPromise() -{ -} - -bool cCombinedPromise::IsCompleted() -{ - return m_left->IsCompleted() || m_right->IsCompleted(); -} - -cCancelablePromise::cCancelablePromise(cPromise* a_wrapped, volatile bool& a_cancel) : - cPromise(), - m_cancel(a_cancel), - m_wrapped(a_wrapped) -{ -} - -cCancelablePromise::~cCancelablePromise () -{ -} - -bool cCancelablePromise::IsCompleted() -{ - return m_cancel || m_wrapped->IsCompleted(); -} - - diff --git a/src/OSSupport/Promise.h b/src/OSSupport/Promise.h deleted file mode 100644 index 83d04860b..000000000 --- a/src/OSSupport/Promise.h +++ /dev/null @@ -1,38 +0,0 @@ -#pragma once - -class cCombinedPromise; - - -class cPromise { - public: - cPromise() {} - virtual ~cPromise () {} - cPromise * WaitFor(cPromise * a_Promise); - cPromise * CancelOn(volatile bool& cancelationtoken); - void Wait(); - virtual bool IsCompleted() = 0; - //TODO:Expose Events for waiting on -}; - -class cCombinedPromise : public cPromise { -public: - cCombinedPromise(cPromise*, cPromise*); - ~cCombinedPromise(); - virtual bool IsCompleted(); -private: - cPromise* m_left; - cPromise* m_right; -}; - -class cCancelablePromise : public cPromise { -public: - cCancelablePromise(cPromise*, volatile bool&); - ~cCancelablePromise(); - virtual bool IsCompleted(); -private: - volatile bool& m_cancel; - cPromise* m_wrapped; -}; - - - diff --git a/src/OSSupport/Queue.h b/src/OSSupport/Queue.h index eb323b067..153e201c1 100644 --- a/src/OSSupport/Queue.h +++ b/src/OSSupport/Queue.h @@ -3,8 +3,6 @@ #include -#include "../OSSupport/Promise.h" - //this empty struct allows function inlining template struct cQueueFuncs @@ -52,6 +50,7 @@ public: if (m_contents.size() == 0) return false; item = m_contents.front(); m_contents.pop_front(); + m_evtRemoved.Set(); return true; } ItemType DequeueItem() @@ -62,10 +61,15 @@ public: cCSUnlock Unlock(m_CS); m_evtAdded.Wait(); } - return m_contents.pop_front(); + ItemType item = m_contents.front(); + m_contents.pop_front(); + m_evtRemoved.Set(); + return item; } - cPromise* BlockTillEmpty() { - return new cEmptyQueuePromise(this); + void BlockTillEmpty() { + //There is a very slight race condition here if the load completes between the check + //and the wait. + while(!(Size() == 0)){m_evtRemoved.Wait();} } //can all be inlined when delete is a noop void Clear() @@ -87,18 +91,12 @@ public: { cCSLock Lock(m_CS); m_contents.remove(item); + m_evtRemoved.Set(); } private: ListType m_contents; cCriticalSection m_CS; cEvent m_evtAdded; - - class cEmptyQueuePromise : public cPromise { - public: - cEmptyQueuePromise(cQueue* a_Queue) : cPromise(), m_Queue(a_Queue) {} - virtual bool IsCompleted() {return m_Queue->Size() != 0;} - private: - cQueue* m_Queue; - }; + cEvent m_evtRemoved; }; -- cgit v1.2.3 From d522619ce2ac05831febedea675d121445dcbdbe Mon Sep 17 00:00:00 2001 From: Tycho Bickerstaff Date: Thu, 2 Jan 2014 17:43:57 +0000 Subject: added documentation --- src/OSSupport/Queue.h | 52 ++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 7 deletions(-) (limited to 'src/OSSupport') diff --git a/src/OSSupport/Queue.h b/src/OSSupport/Queue.h index 153e201c1..1f0c19f40 100644 --- a/src/OSSupport/Queue.h +++ b/src/OSSupport/Queue.h @@ -1,34 +1,55 @@ +// Queue.h + +// Implements the cQueue class representing a thread safe queue + #pragma once #include -//this empty struct allows function inlining +/* +Usage: +To use the callback functions Delete and Combine create a class with the two +methods and pass it as a second template parameter to cQueue. The class does +not need to inherit cQueueFuncs but you do so to document the classes purpose. +The second template parmeter is optional if not overriding the callback +functions +*/ + +// this empty struct allows for the callback functions to be inlined template struct cQueueFuncs { public: + // Called when an Item is deleted form the queue without being returned static void Delete(T) {}; - static void Combine(T&, const T) {}; + // Called when an Item is inserted with EnqueueItemIfNotPresent and + // there is another equal value already inserted + static void Combine(T& a_existing, const T a_new) {}; }; template > class cQueue { - +// internal typedef for a List of Items typedef typename std::list ListType; -//magic typedef to persuade clang that the iterator is a type +// magic typedef to persuade clang that the iterator is a type typedef typename ListType::iterator iterator; public: cQueue() {} ~cQueue() {} + // Enqueues an item to the queue, may block if other threads are accessing + // the queue. void EnqueueItem(ItemType a_item) { cCSLock Lock(m_CS); m_contents.push_back(a_item); m_evtAdded.Set(); } + + // Enqueues an item to the queue if not already present as determined with + // operator ==. Will block other threads from accessing the queue. void EnqueueItemIfNotPresent(ItemType a_item) { cCSLock Lock(m_CS); @@ -44,6 +65,11 @@ public: m_contents.push_back(a_item); m_evtAdded.Set(); } + + // Dequeues an Item from the queue if any are present, provides no + // guarantees about success if the list is empty but an item is enqueued at + // the same time. Returns true if successful. Value of item is undefined if + // Dequeuing was unsuccessful. bool TryDequeueItem(ItemType& item) { cCSLock Lock(m_CS); @@ -53,6 +79,8 @@ public: m_evtRemoved.Set(); return true; } + + // Dequeues an Item from the Queue, blocking until an Item is Available. ItemType DequeueItem() { cCSLock Lock(m_CS); @@ -66,12 +94,17 @@ public: m_evtRemoved.Set(); return item; } + + // Blocks Until the queue is Empty, Has a slight race condition which may + // cause it to miss the queue being empty. void BlockTillEmpty() { - //There is a very slight race condition here if the load completes between the check - //and the wait. + // There is a very slight race condition here if the load completes between the check + // and the wait. while(!(Size() == 0)){m_evtRemoved.Wait();} } - //can all be inlined when delete is a noop + + // Removes all Items from the Queue, calling Delete on each of them. + // can all be inlined when delete is a noop void Clear() { cCSLock Lock(m_CS); @@ -82,11 +115,16 @@ public: m_contents.pop_front(); } } + + // Returns the Size at time of being called + // Do not use to detirmine weather to call DequeueItem, use TryDequeue instead size_t Size() { cCSLock Lock(m_CS); return m_contents.size(); } + + // Removes an Item from the queue bool Remove(ItemType item) { cCSLock Lock(m_CS); -- cgit v1.2.3 From 6f3c5b806eb59e2610f8bac9ad3fff24994609c9 Mon Sep 17 00:00:00 2001 From: Tycho Bickerstaff Date: Fri, 3 Jan 2014 11:22:01 +0000 Subject: implement xsofts recommendations --- src/OSSupport/Queue.h | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) (limited to 'src/OSSupport') diff --git a/src/OSSupport/Queue.h b/src/OSSupport/Queue.h index 1f0c19f40..65f9bd258 100644 --- a/src/OSSupport/Queue.h +++ b/src/OSSupport/Queue.h @@ -5,15 +5,18 @@ #pragma once -#include - /* +Items can be added multiple times to a queue, there are two functions for +adding, EnqueueItem() and EnqueueItemIfNotPresent(). The first one always +enqueues the specified item, the second one checks if the item is already +present and only queues it if it isn't. + Usage: -To use the callback functions Delete and Combine create a class with the two -methods and pass it as a second template parameter to cQueue. The class does -not need to inherit cQueueFuncs but you do so to document the classes purpose. -The second template parmeter is optional if not overriding the callback -functions +To create a queue of type T, instantiate a cQueue object. You can also +modify the behavior of the queue when deleting items and when adding items +that are already in the queue by providing a second parameter, a class that +implements the functions Delete() and Combine(). An example is given in +cQueueFuncs and is used as the default behavior. */ // this empty struct allows for the callback functions to be inlined @@ -25,7 +28,7 @@ struct cQueueFuncs static void Delete(T) {}; // Called when an Item is inserted with EnqueueItemIfNotPresent and // there is another equal value already inserted - static void Combine(T& a_existing, const T a_new) {}; + static void Combine(T& a_existing, const T& a_new) {}; }; template > @@ -73,7 +76,10 @@ public: bool TryDequeueItem(ItemType& item) { cCSLock Lock(m_CS); - if (m_contents.size() == 0) return false; + if (m_contents.size() == 0) + { + return false; + } item = m_contents.front(); m_contents.pop_front(); m_evtRemoved.Set(); -- cgit v1.2.3 From 0e8bb3bf415e4c0fe6c8bd0aa06dc2d9af2823c4 Mon Sep 17 00:00:00 2001 From: Tycho Date: Fri, 3 Jan 2014 08:34:41 -0800 Subject: fixed failure to return a value from Remove --- src/OSSupport/Queue.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/OSSupport') diff --git a/src/OSSupport/Queue.h b/src/OSSupport/Queue.h index 65f9bd258..bf6d7e451 100644 --- a/src/OSSupport/Queue.h +++ b/src/OSSupport/Queue.h @@ -134,8 +134,8 @@ public: bool Remove(ItemType item) { cCSLock Lock(m_CS); - m_contents.remove(item); m_evtRemoved.Set(); + return m_contents.remove(item); } private: -- cgit v1.2.3 From 14ec68d8d309d3fdf8e0af47196b1cf8609d017d Mon Sep 17 00:00:00 2001 From: Tycho Date: Fri, 3 Jan 2014 08:49:14 -0800 Subject: actual fix --- src/OSSupport/Queue.h | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'src/OSSupport') diff --git a/src/OSSupport/Queue.h b/src/OSSupport/Queue.h index bf6d7e451..fc942b3e1 100644 --- a/src/OSSupport/Queue.h +++ b/src/OSSupport/Queue.h @@ -134,8 +134,15 @@ public: bool Remove(ItemType item) { cCSLock Lock(m_CS); - m_evtRemoved.Set(); - return m_contents.remove(item); + for (iterator itr = m_contents.begin(); itr != m_contents.end(); ++itr) + { + if((*itr) == a_item) { + m_contents.erase(itr); + m_evtRemoved.Set(); + return true; + } + } + return false; } private: -- cgit v1.2.3 From 13bbb3d99dee8041d47279cae3484c3083491f18 Mon Sep 17 00:00:00 2001 From: Tycho Date: Fri, 3 Jan 2014 08:56:20 -0800 Subject: derp --- src/OSSupport/Queue.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/OSSupport') diff --git a/src/OSSupport/Queue.h b/src/OSSupport/Queue.h index fc942b3e1..ab42c871e 100644 --- a/src/OSSupport/Queue.h +++ b/src/OSSupport/Queue.h @@ -131,7 +131,7 @@ public: } // Removes an Item from the queue - bool Remove(ItemType item) + bool Remove(ItemType a_item) { cCSLock Lock(m_CS); for (iterator itr = m_contents.begin(); itr != m_contents.end(); ++itr) -- cgit v1.2.3 From d26c0e3815a8e3c38447eeb65b052a5d7c43da73 Mon Sep 17 00:00:00 2001 From: Tycho Date: Fri, 3 Jan 2014 09:42:35 -0800 Subject: Fixed Documentation --- src/OSSupport/Queue.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'src/OSSupport') diff --git a/src/OSSupport/Queue.h b/src/OSSupport/Queue.h index ab42c871e..cde26e415 100644 --- a/src/OSSupport/Queue.h +++ b/src/OSSupport/Queue.h @@ -69,10 +69,8 @@ public: m_evtAdded.Set(); } - // Dequeues an Item from the queue if any are present, provides no - // guarantees about success if the list is empty but an item is enqueued at - // the same time. Returns true if successful. Value of item is undefined if - // Dequeuing was unsuccessful. + // Dequeues an Item from the queue if any are present. Returns true if + // successful. Value of item is undefined if Dequeuing was unsuccessful. bool TryDequeueItem(ItemType& item) { cCSLock Lock(m_CS); -- cgit v1.2.3 From 0a712931b1b547c7e7702ced73251e23dbc7737a Mon Sep 17 00:00:00 2001 From: madmaxoft Date: Sun, 5 Jan 2014 15:15:44 +0100 Subject: Fixed a race condition in the cQueue class. Fixes #505. --- src/OSSupport/Queue.h | 145 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 86 insertions(+), 59 deletions(-) (limited to 'src/OSSupport') diff --git a/src/OSSupport/Queue.h b/src/OSSupport/Queue.h index cde26e415..6c3d58295 100644 --- a/src/OSSupport/Queue.h +++ b/src/OSSupport/Queue.h @@ -19,123 +19,139 @@ implements the functions Delete() and Combine(). An example is given in cQueueFuncs and is used as the default behavior. */ -// this empty struct allows for the callback functions to be inlined +/// This empty struct allows for the callback functions to be inlined template struct cQueueFuncs { - public: - // Called when an Item is deleted form the queue without being returned - static void Delete(T) {}; - // Called when an Item is inserted with EnqueueItemIfNotPresent and - // there is another equal value already inserted - static void Combine(T& a_existing, const T& a_new) {}; +public: + + /// Called when an Item is deleted from the queue without being returned + static void Delete(T) {}; + + /// Called when an Item is inserted with EnqueueItemIfNotPresent and there is another equal value already inserted + static void Combine(T & a_existing, const T & a_new) {}; }; -template > + + + + +template > class cQueue { -// internal typedef for a List of Items -typedef typename std::list ListType; -// magic typedef to persuade clang that the iterator is a type -typedef typename ListType::iterator iterator; + // The actual storage type for the queue + typedef typename std::list QueueType; + + // Make iterator an alias for the QueueType's iterator + typedef typename QueueType::iterator iterator; + public: cQueue() {} ~cQueue() {} - // Enqueues an item to the queue, may block if other threads are accessing - // the queue. - void EnqueueItem(ItemType a_item) + + /// Enqueues an item to the queue, may block if other threads are accessing the queue. + void EnqueueItem(ItemType a_Item) { cCSLock Lock(m_CS); - m_contents.push_back(a_item); + m_Contents.push_back(a_Item); m_evtAdded.Set(); } - // Enqueues an item to the queue if not already present as determined with - // operator ==. Will block other threads from accessing the queue. - void EnqueueItemIfNotPresent(ItemType a_item) + + /// Enqueues an item in the queue if not already present (as determined by operator ==). Blocks other threads from accessing the queue. + void EnqueueItemIfNotPresent(ItemType a_Item) { cCSLock Lock(m_CS); - for (iterator itr = m_contents.begin(); itr != m_contents.end(); ++itr) + for (iterator itr = m_Contents.begin(); itr != m_Contents.end(); ++itr) { - if((*itr) == a_item) { - Funcs funcTable; - funcTable.Combine(*itr,a_item); + if ((*itr) == a_Item) + { + Funcs::Combine(*itr, a_Item); return; } } - m_contents.push_back(a_item); + m_Contents.push_back(a_Item); m_evtAdded.Set(); } - // Dequeues an Item from the queue if any are present. Returns true if - // successful. Value of item is undefined if Dequeuing was unsuccessful. - bool TryDequeueItem(ItemType& item) + + /// Dequeues an item from the queue if any are present. + /// Returns true if successful. Value of item is undefined if dequeuing was unsuccessful. + bool TryDequeueItem(ItemType & item) { cCSLock Lock(m_CS); - if (m_contents.size() == 0) + if (m_Contents.size() == 0) { return false; } - item = m_contents.front(); - m_contents.pop_front(); + item = m_Contents.front(); + m_Contents.pop_front(); m_evtRemoved.Set(); return true; } - // Dequeues an Item from the Queue, blocking until an Item is Available. - ItemType DequeueItem() + + /// Dequeues an item from the queue, blocking until an item is available. + ItemType DequeueItem(void) { cCSLock Lock(m_CS); - while (m_contents.size() == 0) + while (m_Contents.size() == 0) { - cCSUnlock Unlock(m_CS); + cCSUnlock Unlock(Lock); m_evtAdded.Wait(); } - ItemType item = m_contents.front(); - m_contents.pop_front(); + ItemType item = m_Contents.front(); + m_Contents.pop_front(); m_evtRemoved.Set(); return item; } - // Blocks Until the queue is Empty, Has a slight race condition which may - // cause it to miss the queue being empty. - void BlockTillEmpty() { - // There is a very slight race condition here if the load completes between the check - // and the wait. - while(!(Size() == 0)){m_evtRemoved.Wait();} + + /// Blocks until the queue is empty. + void BlockTillEmpty(void) + { + cCSLock Lock(m_CS); + while (!m_Contents.empty()) + { + cCSUnlock Unlock(Lock); + m_evtRemoved.Wait(); + } } - // Removes all Items from the Queue, calling Delete on each of them. - // can all be inlined when delete is a noop - void Clear() + + /// Removes all Items from the Queue, calling Delete on each of them. + void Clear(void) { cCSLock Lock(m_CS); - Funcs funcTable; - while (!m_contents.empty()) + while (!m_Contents.empty()) { - funcTable.Delete(m_contents.front()); - m_contents.pop_front(); + Funcs::Delete(m_Contents.front()); + m_Contents.pop_front(); } } - // Returns the Size at time of being called - // Do not use to detirmine weather to call DequeueItem, use TryDequeue instead - size_t Size() + + /// Returns the size at time of being called. + /// Do not use to determine whether to call DequeueItem(), use TryDequeueItem() instead + size_t Size(void) { cCSLock Lock(m_CS); - return m_contents.size(); + return m_Contents.size(); } - // Removes an Item from the queue - bool Remove(ItemType a_item) + + /// Removes the item from the queue. If there are multiple such items, only the first one is removed. + /// Returns true if the item has been removed, false if no such item found. + bool Remove(ItemType a_Item) { cCSLock Lock(m_CS); - for (iterator itr = m_contents.begin(); itr != m_contents.end(); ++itr) + for (iterator itr = m_Contents.begin(); itr != m_Contents.end(); ++itr) { - if((*itr) == a_item) { - m_contents.erase(itr); + if ((*itr) == a_Item) + { + m_Contents.erase(itr); m_evtRemoved.Set(); return true; } @@ -144,8 +160,19 @@ public: } private: - ListType m_contents; + /// The contents of the queue + QueueType m_Contents; + + /// Mutex that protects access to the queue contents cCriticalSection m_CS; + + /// Event that is signalled when an item is added cEvent m_evtAdded; + + /// Event that is signalled when an item is removed (both dequeued or erased) cEvent m_evtRemoved; }; + + + + -- cgit v1.2.3 From 487c1a24de3c375365c77232a47b99ddef0de68b Mon Sep 17 00:00:00 2001 From: Diusrex Date: Sun, 5 Jan 2014 15:08:30 -0700 Subject: Added fake functions into cCriticalSection because of the change to ASSERT --- src/OSSupport/CriticalSection.h | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src/OSSupport') diff --git a/src/OSSupport/CriticalSection.h b/src/OSSupport/CriticalSection.h index 1bfe81439..73a71f5e1 100644 --- a/src/OSSupport/CriticalSection.h +++ b/src/OSSupport/CriticalSection.h @@ -14,9 +14,14 @@ public: void Lock(void); void Unlock(void); + // IsLocked/IsLockedByCurrentThread are only used in ASSERT statements, but because of the changes with ASSERT they must always be defined + // The fake versions (in Release) will not effect the program in any way #ifdef _DEBUG bool IsLocked(void); bool IsLockedByCurrentThread(void); + #else + bool IsLocked(void) { return false; } + bool IsLockedByCurrentThread(void) { return false; } #endif // _DEBUG private: -- cgit v1.2.3