The reader_writer multithreading problem with locks results in a crash?

Question

I am trying to implement sample code for Reader writer problem but the below code is crashing in Reader API. In the code below, count is my shared resource between reader and writer.

Here is the exception that throws in gcc compiler :

terminate called after throwing an instance of 'std::system_error'
  what():  Operation not permitted

What's wrong in the code?

#include<iostream>
#include <thread>
#include <mutex>
using namespace std;

class ReaderAndWriter
{
    mutex rmu;
    mutex wmu;
    int count = 20;
    int readCount{ 0 };

public:
    

    void Reader()
    {
        unique_lock<mutex> readLocker(rmu, std::defer_lock);
        unique_lock<mutex> writeLocker(wmu, std::defer_lock);

        readLocker.lock();
        readCount++;
        if (readCount == 1)
        {
            writeLocker.lock();
        }
        readLocker.unlock();

        cout << "Reader " << count << endl;

        readLocker.lock();
        readCount--;
        if (readCount == 0) 
        {
            writeLocker.unlock();
        }
        readLocker.unlock();
        
    }

    void Writer()
    {
        unique_lock<mutex> locker(wmu); 
        count++;
        cout << "writer " << count << endl;
        locker.unlock();
    }

    void run()
    {
        std::thread reader1(&ReaderAndWriter::Reader, this);
        std::thread reader2(&ReaderAndWriter::Reader, this);
        std::thread reader3(&ReaderAndWriter::Reader, this);

        std::thread writer1(&ReaderAndWriter::Writer, this);
        std::thread writer2(&ReaderAndWriter::Writer, this);

        reader1.join();
        reader2.join();
        reader3.join();

        writer1.join();
        writer2.join();

        cout << "Success" << endl;
    }
};

int main()
{
    ReaderAndWriter rw;
    rw.run();
    return true;
}

Reader writer locks have default support in C++. See [std::shared_lock](https://en.cppreference.com/w/cpp/thread/shared_lock), also see this [SO question](https://stackoverflow.com/questions/244316/reader-writer-locks-in-c) And do not use `using namespace std;` As to your exception that is probably the result of trying to acquire an already locked lock std::mutex and also why std::shared_mutex should be used for reader writer locks. — Pepijn Kramer, Nov 06 '22 at 09:55
_"exception that throws in gcc compiler"_ - that is a runtime error, not a compiler error. — Clifford, Nov 06 '22 at 11:11

user17732522 · Answer 1 · 2022-11-06T15:41:43.870

Consider the following possible serial execution order which is permitted by your synchronization:

Suppose reader thread 1 passes through the first readLocker section first, incrementing readCount to 1 and taking the writeLocker lock. Then reader thread 2 passes through the same section, incrementing readCount to 2.

Then thread 1 passes through the second readLocker section and doesn't unlock writeLocker because readCount decrements to 1, not 0. Then reader thread 2 passes through the same section and does try to unlock writeLocker because readCount is now decremented to 0, but since it had never taken the lock, trying that has undefined behavior. An exception is an allowed outcome of that.

As pointed out by @SolomonSlow in a comment under this answer, this can be remedied by using a semaphore instead of a mutex for wmu. Since C++20 there are semaphores in the standard library. Specifically you could use a std::binary_semaphore initialized to 1. Then acquire can act similar to lock and release similar to unlock, but it wouldn't matter whether the same thread that called acquire will call release.

As also pointed out in a comment under the question there is already std::shared_mutex and std::shared_lock in the standard library which may be used to manage exclusive as well as shared access without having to implement it with other primitives yourself.

Good catch! You might point out though that there's an easy fix for this problem in C++20. Use a [_semaphore_](https://en.cppreference.com/w/cpp/thread/counting_semaphore) instead of a mutex for the "write" lock. A binary semaphore is very nearly the same thing as a mutex, except that it is allowed for one thread to "lock" it and a different thread to "unlock" it. I would not be surprised if OP's code example were adapted from some older example that explicitly used old-school semaphores instead of modern mutexes. — Solomon Slow, Nov 06 '22 at 14:36

The reader_writer multithreading problem with locks results in a crash?

1 Answers1