Hello, I'm lacking a boost semaphore, so I tried to roll my own. Being carefull with threads, I present here my version, hoping you gurus will spot any flaw. class sem { sem(unsigned count): m_count(count) {} void post() { boost::mutex::scoped_lock lock(m_mutex); ++m_count; m_cond_.notify_one(); } void wait() { boost::mutex::scoped_lock lock(m_mutex); while ( m_count==0 ) m_cond.wait( lock ); --m_count; } private: unsigned m_count; boost::mutex m_mutex; boost::condition m_cond; }; Objective : I need a semaphore to sync many readers to a single input queue. If you know another implementation or solution to this specific problem, your help is welcome. Thank you.
Amerio wrote:
Hello,
I'm lacking a boost semaphore, so I tried to roll my own. Being carefull with threads, I present here my version, hoping you gurus will spot any flaw.
class sem { sem(unsigned count): m_count(count) {}
void post() { boost::mutex::scoped_lock lock(m_mutex); ++m_count; m_cond_.notify_one(); }
void wait() { boost::mutex::scoped_lock lock(m_mutex); while ( m_count==0 ) m_cond.wait( lock ); --m_count; }
private: unsigned m_count; boost::mutex m_mutex; boost::condition m_cond; };
Objective : I need a semaphore to sync many readers to a single input queue. If you know another implementation or solution to this specific problem, your help is welcome.
Looks almost correct. But as you likely will have multiple waiters, I suggest using m_cond_.notify_all() instead. You might also consider to obtain a book on pthreads (if you do not already own), where you will find that the semaphore is just a special case of (condvar, mutex, predicate) triple. I also recommend to look at the "condvar" example of boost.thread, which shows a simple queue implementation (albeit for single reader/writer). Roland
void post() { boost::mutex::scoped_lock lock(m_mutex); ++m_count; m_cond_.notify_one(); }
Objective : I need a semaphore to sync many readers to a single input queue. If you know another implementation or solution to this specific problem, your help is welcome.
Looks almost correct. But as you likely will have multiple waiters, I suggest using m_cond_.notify_all() instead.
This one I don't understand. When my sem is incremented by one, I want to wake up only one reader (as there is only one new entry to be read). Why would I want to wake up all of them ? As I see it, one post means one new entry exactly.
I also recommend to look at the "condvar" example of boost.thread, which shows a simple queue implementation (albeit for single reader/writer).
I find their example a little tricky. I specially miss a multiple reader/writer example. Moreover, the loop around the cond.wait() is tricky at first and should get more attention. In my own example, I was also wondering if I got the "while" loop right : void wait() { boost::mutex::scoped_lock lock(m_mutex); while ( m_count==0 ) m_cond.wait( lock ); // is cond test ok ? --m_count; } Thanks for your insights.
Amerio wrote:
Looks almost correct. But as you likely will have multiple waiters, I suggest using m_cond_.notify_all() instead.
This one I don't understand. When my sem is incremented by one, I want to wake up only one reader (as there is only one new entry to be read). Why would I want to wake up all of them ? As I see it, one post means one new entry exactly.
Ooops, of course you can't understand, because my answer was wrong in this respect. Sorry for this. It was kind of lazyness which led me to this answer. It cannot happen, that the counter increases while you are waiting since you are notifying on each increment. So your example seems correct to me.
I also recommend to look at the "condvar" example of boost.thread, which shows a simple queue implementation (albeit for single reader/writer).
I find their example a little tricky. I specially miss a multiple reader/writer example. Moreover, the loop around the cond.wait() is tricky at first and should get more attention. In my own example, I was also wondering if I got the "while" loop right : void wait() { boost::mutex::scoped_lock lock(m_mutex); while ( m_count==0 ) m_cond.wait( lock ); // is cond test ok ? --m_count; }
Hopefully beeing more careful now: Using locks and condvars this way is exactly the recommended usage pattern. This also takes care of so called 'spurious' wakeups which might take place dependant on the implementation of the wait. This also is the reason why notify_all would not have done harm other than beeing of slower performance. Yet another tought: When you have returned from your wait function you will need another lock to access your queue in order to retrieve an entry. On the other hand when returning from m_cond.wait you are the holder of lock. So if you use the m_mutex also to protect your queue you save one locking operation. Consider doing away with the semaphore and integrate the condvar, mutex pair into your queue. Think of a semaphore only beeing a special case of (condvar,mutex,counter) triple. Roland
Roland Schwarz <roland.schwarz@chello.at> writes: | Yet another tought: | When you have returned from your wait function you will need another lock to | access your queue in order to retrieve an entry. On the other hand | when returning | from m_cond.wait you are the holder of lock. So if you use the m_mutex | also to | protect your queue you save one locking operation. | Consider doing away with the semaphore and integrate the condvar, mutex pair | into your queue. | Think of a semaphore only beeing a special case of | (condvar,mutex,counter) triple. Similar to this then I suppose? I am sure it can be improved, but it has worked really well for me. -- Lgb
Yet another tought: When you have returned from your wait function you will need another lock to access your queue in order to retrieve an entry. On the other hand when returning from m_cond.wait you are the holder of lock. So if you use the m_mutex also to protect your queue you save one locking operation.
I thought about this, too. Yet, the boost doc says that wait() will UNLOCK then RELOCK the mutex. Couldn't it be that in between someone else take the lock ? The doc is quite fuzzy around this...
Consider doing away with the semaphore and integrate the condvar, mutex pair into your queue. Think of a semaphore only beeing a special case of (condvar,mutex,counter) triple.
I may try this approach. Thank you.
Amerio wrote:
I thought about this, too. Yet, the boost doc says that wait() will UNLOCK then RELOCK the mutex. Couldn't it be that in between someone else take the lock ? The doc is quite fuzzy around this...
This is the _key_ point of condvar mutex use. Because it is guaranteed noone else can go in between it is by no means easy to implement it e.g. on the windows platform. Luckily however some clever guys have solved this problem. :-) Again, I highly recommend to get a book on pthreads. You will find a lot of useful information which also applies to boost threads. Roland
participants (3)
-
Amerio -
larsbj＠gullik.net -
Roland Schwarz