2

Background - (skip this if you want to)

I've written a few multiple reader/single writer (MRSW) implementations before, but they have used either native or lower-level synchronication mechanisms than those provided by C++11 and later. I'm currently updating some older code to be more "modern C++" oriented, as well as more portable, and looked into solutions based on the C++1x standards. One that looked especially interesting due to its simplicity and brevity (code copied below, full thread at How to make a multiple-read/single-write lock from more basic synchronization primitives?) was posted here on SO.

// from link posted above, code originally by qqibrow, edited by sbi at Stack Overflow
class RWLock {
    public:
        RWLock()
            : shared()
            , readerQ(), writerQ()
            , active_readers(0), waiting_writers(0), active_writers(0)
        {}

        void ReadLock() {
            std::unique_lock<std::mutex> lk(shared);
            while( waiting_writers != 0 )                       // QUESTION BELOW
                readerQ.wait(lk);
            ++active_readers;
            lk.unlock();
        }

        void ReadUnlock() {
            std::unique_lock<std::mutex> lk(shared);
            --active_readers;
            lk.unlock();
            writerQ.notify_one();
        }

        void WriteLock() {
            std::unique_lock<std::mutex> lk(shared);
            ++waiting_writers;
            while( active_readers != 0 || active_writers != 0 )   // QUESTION BELOW
                writerQ.wait(lk);
            ++active_writers;
            lk.unlock();
        }

        void WriteUnlock() {
            std::unique_lock<std::mutex> lk(shared);
            --waiting_writers;
            --active_writers;
            if(waiting_writers > 0)
                writerQ.notify_one();
            else
                readerQ.notify_all();
            lk.unlock();
        }

    private:
        std::mutex              shared;
        std::condition_variable readerQ;
        std::condition_variable writerQ;
        int                     active_readers;
        int                     waiting_writers;
        int                     active_writers;
};

My Question

My question is about the semantics of mutex locking and access for this MRSW implementation, especially std::unique_lock (specifically, the lines marked in comments in the above code). Using traditional lower-level synchronization, after ReadLock() or WriteLock() acquires exclusive ownwership to the sync object, how can another thread modify waiting_writers, active_readers or active_writers while the lock is acquired? As I mentioned, I've used the two-gate approach before for MRSW, but the simplicity of the above implementation makes me feel like it's either too straightforward to be correct, or I am missing something about the semantics of exclusive locking in C++1x.

I've looked at relevant references, but the semantics for a case like this are not particularly clear (e.g http://en.cppreference.com/w/cpp/thread/unique_lock).

It seems the unlock needs to be handled differently, and I'm probably missing something, but I'd really like some clarity about what it is.

Thanks!

frasnian
  • 1,973
  • 1
  • 14
  • 24
  • Why aren't `active_readers`, `active_writers`, atomic? – user9335240 Apr 16 '18 at 21:17
  • If you are already using C++11, what's wrong with `std::shared_mutex`? – SergeyA Apr 16 '18 at 21:26
  • @user9335240 - If you're locking the entire object using a mutex, they shouldn't need to be, but my question is how can they be accessed if the object is locked? It's about the semantics of `unique_lock`. – frasnian Apr 16 '18 at 21:33
  • @SergeyA This is basically my question - how can `std::mutex` be correct here, based on how member variables are accessed after obtaining an exclusive lock? I didn't write the code, just asking how it could possibly be right. – frasnian Apr 16 '18 at 21:36
  • 2
    `std::condition_variable` unlocks the mutex while waiting, and relocks when woken up. [It's baked into `wait` call](http://en.cppreference.com/w/cpp/thread/condition_variable/wait) - that's why it takes `std::unique_lock` – Igor Tandetnik Apr 16 '18 at 21:36
  • @IgorTandetnik - thanks, man. That's exactly what I needed - I was looking in the wrong place. Please post that as an answer so I can give credit where credit is due. – frasnian Apr 16 '18 at 21:40
  • 1
    @frasnian: Also note that you can use predicates with `wait()` to avoid having to use manual loops. And you don't have to explicitly `unlock` a mutex, it will unlock when it goes out of scope. `void ReadLock() { std::unique_lock lk(shared); readerQ.wait(lk, [&](){ return waiting_writers == 0; }); ++active_readers; }` ... `void WriteLock() { std::unique_lock lk(shared); ++waiting_writers; writerQ.wait(lk, [&](){ return active_readers == 0 && active_writers == 0 }); ++active_writers; }` – Remy Lebeau Apr 16 '18 at 23:03
  • @RemyLebeau interesting, but as lambda functions often (usually, based on how they're implemented) create a whole additional layer, I'd have to look at code actually generated and performance. Will definitely check it out, though, thanks. – frasnian Apr 17 '18 at 18:44
  • @frasnian using a lambda is just for convenience. The predicate you pass to `wait()` doesn't have to be a lambda. The important thing is that a conditional variable suffers from spurious wakes, so a loop is needed to ensure the desired wake condition is actually met before you continue with your processing inside the lock. `wait()` can do that loop for you, calling the predicate on each loop iteration, so it will not exit until the wake condition is met. Makes coding a little simpler and safer – Remy Lebeau Apr 17 '18 at 20:49
  • @RemyLebeau - isn't that a distinction without a difference? I don't see how a spurious wake from a condition variable is mitigated by a predicate in this case - it seems to just drop it one level (please correct me if I'm mistaken about this). – frasnian Apr 17 '18 at 22:18
  • @frasnian when using a predicate, `wait()` handles the spurious wake for you, and does not exit back to your code until a true wake occurs. Without a predicate, `wait()` exits on any kind of wake, and you have to handle spurious wakes yourself by calling `wait()` again. – Remy Lebeau Apr 17 '18 at 23:50

0 Answers0