How can I implement a C++ Reader-Writer lock using a single unlock method, which can be called be a reader or writer?

Question

I'm working on a project, which requires the use of specific OS abstractions and I need to implement a reader-writer lock using their semaphore and mutex. I currently, have a setup in the format:

class ReadWriteLock
{
public:
   ReadWriteLock(uint32_t maxReaders);
   ~ReadWriteLock();
   uint32_t GetMaxReaders() const;
   eResult  GetReadLock(int32_t timeout);
   eResult  GetWriteLock(int32_t timeout);
   eResult  Unlock();

private:
   uint32_t m_MaxReaders;
   Mutex* m_WriterMutex;
   Semaphore* m_ReaderSemaphore;

};

In this implementation I need to use this Unlock method to either unlock the writer and release all reader semaphore slots, or to simply unleash a reader semaphore slot, however, I am struggling as I cannot think of an implementation, which will be work in all cases. How can I make this work in the given setup? I know it is possible as POSIX was able to implement a universal unlock method in their implementation, but I cannot find any indication of how that was done, so would appreciate any information people can share.

Note that I cannot use C++11 or other OS primitives.

I strongly suggest that you name and design your functions in accordance with the `Shared Mutex` concept: http://en.cppreference.com/w/cpp/concept/SharedMutex. That way you can use RAII locking wrappers, `unique_lock` and `shared_lock` with your class. — Nir Friedman, Mar 24 '17 at 15:59

score 2 · Accepted Answer · edited May 23 '17 at 11:54

Well, define two functions UnlockRead and UnlockWrite.

I believe you do not need both accesses (Write/Read) at the same time in the same place. So what I am proposing is to have two other classes for locking access:

class ReadWriteAccess
{
public:
   ReadWriteAccess(uint32_t maxReaders);
   ~ReadWriteAccess();
   uint32_t GetMaxReaders() const;
   uint32_t GetMaxReaders() const;
   eResult  GetReadLock(int32_t timeout);
   eResult  GetWriteLock(int32_t timeout);
   eResult  UnlockWrite();
   eResult  UnlockRead();

private:
   uint32_t m_MaxReaders;
   Mutex* m_WriterMutex;
   Semaphore* m_ReaderSemaphore;

};

And have separate classes for read and write lock and use RAII to be always on safe side:

class ReadLock
{
public:
    ReadLock(ReadWriteAccess& access, int32_t timeout) : access(access) 
    {
        result = access.GetReadLock(timeout);
    }
    eResult getResult() const { return result; }
    ~ReadLock()
    {
        if (result)
            access.UnlockRead();
    }
private:
    ReadWriteAccess& access;
    eResult  result;
};

and use like this:

T someResource;
ReadWriteAccess someResourceGuard;

void someFunction()
{
    ReadLock lock(someResourceGuard);
    if (lock.getResult())
       cout << someResource; // it is safe to read something from resource
}

Of course, the very similar implementation you can easily write by yourself for WriteLock

Since OP insisted in comments to have "one" Unlock - please consider the drawbacks:

Assume it is implemented some kind of stack of last calls to Lock functions:

class ReadWriteLock
{
public:
   ReadWriteLock(uint32_t maxReaders);
   ~ReadWriteLock();
   uint32_t GetMaxReaders() const;
   eResult  GetReadLock(int32_t timeout)
   {
       eResult result = GetReadLockImpl(timestamp);
       if (result)
           lockStack.push(READ);
   }
   eResult  GetWriteLock(int32_t timeout)
   {
       eResult result = GetWriteLockImpl(timestamp);
       if (result)
           lockStack.push(WRITE);
   }
   eResult  Unlock()
   {
       LastLockMode lockMode = lockStack.top();
       lockStack.pop();
       if (lockMode == READ) 
           UnlockReadImpl();
       else
           UnlockWriteImpl();
   }

private:
   uint32_t m_MaxReaders;
   Mutex* m_WriterMutex;
   Semaphore* m_ReaderSemaphore;

    enum Mode { READ, WRITE };
    std::stack<Mode> lockStack;
};

But the above would work only in one-thread application. And one-thread application never need any locks.

So - you have to have multi-thread stack - like:

template <typename Value>
class MultiThreadStack
{
public:
    void push(Value)
    {
       stackPerThread[getThreadId()].push(value);
    }
    Value top()
    {
       return stackPerThread[getThreadId()].top();
    }
    void pop()
    {
       stackPerThread[getThreadId()].pop();
    }
private:
    ThreadId getThreadId() { return /* your system way to get thread id*/; }
    std::map<ThreadId, std::stack<Value>> stackPerThread;
};

So use this MultiThreadStack not std::stack in ReadWriteLock.

But, the std::map above would need ReadWriteLock to lock access to it from multuple threads - so, well, either you know all your threads before you start using this stuff (preregistration) or you end up in the same problem as described here. So my advice - if you can - change your design.

If you write the interface to the standard Shared Mutex concept though, you can just reuse `unique_lock` and `shared_lock` instead of having to write `ReadLock` from scratch, not to mention making the code generic. — Nir Friedman, Mar 24 '17 at 16:01
I appreciate the answer, but as I specified above I need a universal unlock method as the code using this will be legacy and will expect to call unlock() and not unlockReader() or unlockWriter() — L-S, Mar 24 '17 at 16:06
@PiotrNycz My mistake. That does reduce the utility but I think it's still worth writing it the same way, one day 11 will be available. Also eases the burden on any developers you hire (that probably use 11). E.g. we're on 11 but have an identical name + interface version of make_unique. — Nir Friedman, Mar 24 '17 at 16:09
@L-S The idea of locked access to resource (either write or read access) is that you in some place (called critical area) call first read-lock, read the resource, then call the unlock-read. You cannot mix write-lock with read-unlock. The best way to do this is to use RAII pattern. If you do not follow this scheme - you are in the real problems. — PiotrNycz, Mar 24 '17 at 16:27
@L-S Of course you can keep some "stack" of calls of "lockread", "lockwrite" in your ReadWriteAccess class - but you would have to do it for each thread... that's really nothing you want to do... — PiotrNycz, Mar 24 '17 at 16:29

score 1 · Answer 2 · answered Mar 24 '17 at 16:52

When acquiring the lock successfully the type is known: either you have many readers running or only one writer, you cannot have both readers and writers running with a validly acquired lock.

So it suffices to store the current lock mode when a lock call succeeds and all following unlock calls (potentially many in case reading permit was provided, only one indeed if writing lock was requested) will be of that mode.

How can I implement a C++ Reader-Writer lock using a single unlock method, which can be called be a reader or writer?

2 Answers2