Implementing Binary Semaphore using atomic<int>-CodePudding

I have written this code to demonstrate Binary Semaphore using only atomic .

1 thread producer will push 100 elements in the queue initially.

later threads 2 and 3 which is the consumer will run in parallel to consume this queue.

The issue is: I can see the same data/element print by both the threads

BinarySemaphore.cpp

std::queue<int> buffer;
int s_data = 1;


struct Semaphore
{
    Semaphore():s_(1)
    {

    }
    void wait()
    {
        while( s_.load() ==  0);    //you will keep waiting here until s_ becomes 1
        s_.fetch_sub(1);
    }

    void signal()
    {
        s_.fetch_add(1);
    }

    private :
    std::atomic<int> s_ ;

};

Semaphore s; 


void producer()
{
    
    while(s_data <= 100)
    {
        s.wait();

        // critical section starts
        {
            
            std::ostringstream oss;
            oss << "Consumer pushing data " << s_data <<endl;

            cout << oss.str();
            
            buffer.push(s_data  );
        }
        // critical section ends

        s.signal();
    }
}

void consumer()
{
    while (1)
    {
        s.wait();
        // critical section starts
        if (!buffer.empty())
        {
            int top = buffer.front();
            buffer.pop();

            std::ostringstream oss;
            oss << "consumer  thread id= " << this_thread::get_id() <<  " reading data = " << top  << endl;
            cout << oss.str();
        }
        // critical section ends
        s.signal();
    }
}




int main()
{
    Semaphore s;

    std::thread prod(producer);

    prod.join();

    std::thread cons1(consumer);
    std::thread cons2(consumer);


    cons1.join();
    cons2.join();

}

CodePudding user response：

If you need to do more then one action on atomic you need check consistency if data was not changed. Other wise you will have a "gap" as point out in other answer.

There is a compare_exchange which should be used for that:

    void wait()
    {
        auto oldValue = s_.load();
        while (oldValue == 0 || !s_.compare_exchange_strong(oldValue, oldValue - 1))
           oldValue = s_.load();
    }

Now if oldValue is out of date oldValue will be updated and new check will be performed (new iteration of loop) and in next iteration condition will be checked again.

CodePudding user response：

You have a "gap" in wait:

void wait()
    {
        while( s_.load() ==  0);    //you will keep waiting here until s_ becomes 1
        s_.fetch_sub(1);
    }

load() and fetch_sub are atomic by themselves but between the while... load() and fetch_sub() there is a gap. Maybe you should use "exchange" (and evaluate the result): https://en.cppreference.com/w/cpp/atomic/atomic/exchange or even better use compare_exchange: https://en.cppreference.com/w/cpp/atomic/atomic/compare_exchange