Implementing a semaphore using std :: mutex

Question

As an exercise for learning, I'm just trying to create a Semaphore class using std::mutex and a few other things provided by the C ++ standard. My semaphore should allow as many readLock() as possible, however writeLock() can only be obtained after all reads are unlocked.

 //Semaphore.h #include <mutex> #include <condition_variable> class Semaphore{ public: Semaphore(); void readLock(); //increments the internal counter void readUnlock(); //decrements the internal counter void writeLock(); //obtains sole ownership. must wait for count==0 first void writeUnlock(); //releases sole ownership. int count; //public for debugging private: std::mutex latch; std::unique_lock<std::mutex> lk; std::condition_variable cv; }; //Semaphore.cpp #include "Semaphore.h" #include <condition_variable> #include <iostream> using namespace std; Semaphore::Semaphore() : lk(latch,std::defer_lock) { count=0; } void Semaphore::readLock(){ latch.lock(); ++count; latch.unlock(); cv.notify_all(); //not sure if this needs to be here? } void Semaphore::readUnlock(){ latch.lock(); --count; latch.unlock(); cv.notify_all(); //not sure if this needs to be here? } void Semaphore::writeLock(){ cv.wait(lk,[this](){ return count==0; }); //why can't std::mutex be used here? } void Semaphore::writeUnlock(){ lk.unlock(); cv.notify_all(); } 

My test program will writeLock() semaphore, start a bunch of threads, and then release the semaphore. Immediately after this, the main thread will again try to writeLock() semaphore. The idea is that when the semaphore becomes unlocked, the threads will readLock() and prevent the main thread from executing until all of them run out. When they all finish and release the semaphore, the main thread can access again. I understand that this may not necessarily happen, but this is one of the cases I'm looking for.

 //Main.cpp #include <iostream> #include <thread> #include "Semaphore.h" using namespace std; Semaphore s; void foo(int n){ cout << "Thread Start" << endl; s.readLock(); this_thread::sleep_for(chrono::seconds(n)); cout << "Thread End" << endl; s.readUnlock(); } int main(){ std::srand(458279); cout << "App Launch" << endl; thread a(foo,rand()%10),b(foo,rand()%10),c(foo,rand()%10),d(foo,rand()%10); s.writeLock(); cout << "Main has it" << endl; a.detach(); b.detach(); c.detach(); d.detach(); this_thread::sleep_for(chrono::seconds(2)); cout << "Main released it" << endl; s.writeUnlock(); s.writeLock(); cout << "Main has it " << s.count << endl; this_thread::sleep_for(chrono::seconds(2)); cout << "Main released it" << endl; s.writeUnlock(); cout << "App End" << endl; system("pause"); //windows, sorry return 0; } 

The program throws an exception saying "unlocking broken mutexes." I think the error is in writeLock() or writeUnlock() , but I'm not sure. Can someone point me in the right direction?

EDIT: during lk initialization, the constructor did not have std::defer_lock , however, it did not fix the error that I received. As mentioned in the comment, this is not a semaphore, and I apologize for the confusion. To repeat the problem, here is the result I get (the things in brackets are just my comments and are not actually output):

 App Launch Thread Start Thread Start Main has it Thread Start Thread Start Thread End (what?) Main released it f:\dd\vctools\crt_bld\self_x86\crt\src\thr\mutex.c(131): unlock of unowned mutex Thread End Thread End Thread End 

Answer 1

This is definitely not a semaphore.

Your Semaphore constructor immediately gets a lock on latch , then you unlock it twice, because writeUnlock() calls lk.unlock() , and the next call to writeLock() tries to wait for the condition variable with the unlocked mutex, i.e. undefined behavior, then you call the next call to writeUnlock() to unlock an unlocked mutex, which is also undefined.

Are you sure the constructor should immediately lock mutexes? I think you want to use std::defer_lock in the constructor, and then lock the mutex in writeLock() .