Proper C++ way of implementing a lock_guard for custom library

Question

O wise interwebs

We have an impasse between two colleagues that we could use your help resolving in the proper C++ way. Basically we have a set of utility classes, two of which are a Mutex and SpinLock class which both have the following abridged interface:

class Mutex {
public:
    Mutex();
    ~Mutex();
    void Lock();
    void Unlock();
    // ...
};

Obviously this is similar to, but differently-cased than the BasicLockable concept used by std::lock_guard, so we want something similar (assume that the Mutex class is immutable in this example; we cannot add the BasicLockable concept to it). Also not all of our compilers for supported platforms are fully c++11 featured, so we cannot just use vanilla c++11 support.

One school of thought is the following implementation of a generic guard class which can be inherited to provide a generic guard class and inherit from it to create a lock-guard class:

template<class T, void (T::*EnterFn)(), void (T::*ExitFn)()>
class Guard
{
public: // copy constructor deleting omitted for brevity
    Guard( T *lock ) : m_lock(lock) { (m_lock->*EnterFn)(); }
    ~Guard();                       { (m_lock->*ExitFn)(); }
private:
    T *m_lock;
};

template<class T>
class LockGuard : public Guard<T, &T::Lock, &T::Unlock>
{
public:
    LockGuard(const T* lock) : Guard<T, &T::Lock, &T::Unlock>(lock) {}
};

The other school of thought is to just implement a simple lockguard:

template<class T>
class LockGuard {
    T* m_lockable;
public:
    LockGuard(const T* lockable) : m_lockable(lockable) { lockable->Lock(); }
    ~LockGuard() { m_lockable->Unlock(); }
};

Which implementation would you choose and why? What is the most proper C++(03, 11, 14, 17) way of implementing it? Is there any inherent value to having a generic Guard class as described above?

Answer 1

I would not want to use method pointers.

Personally, I'd want to move towards the C++11 standard tools as much as possible. So I'd write an adapter.

template<class T>
struct lock_adapter {
  T* t = nullptr;
  void lock() { t->Lock(); }
  void unlock() { t->Unlock(); }
  lock_adapter( T& tin ):t(std::addressof(tin)) {}
  // default some stuff if you like
};

template<class T>
struct adapted_unique_lock:
  private lock_adapter<T>,
  std::unique_lock< lock_adapter<T> >
{
  template<class...Args>
  adapted_unique_lock(T& t, Args&&...):
    lock_adapter<T>(t),
    std::unique_lock< lock_adapter<T> >( *this, std::forward<Args>(args)... )
  {}
  adapted_unique_lock(adapted_unique_lock&&)=delete; // sadly
  friend void swap( adapted_unique_lock&, adapted_unique_lock& ) = delete; // ditto
};

now adapted_unique_lock has a restricted set of functionality from a std::unique_lock.

It cannot be moved, as the unique_lock holds a pointer to this inside its implementation and does not reseat it.

Note that the richness of the entire unique_lock constructor set is available.

Functions that return adapted unique locks must store their return values in something like auto&& references until end of scope, and you cannot return them through chains until C++17.

But any code using adapted_unique_lock can be swapped to use unique_lock once T has been changed to support .lock() and .unlock(). This moves your code base towards being more standard C++11, rather than bespoke.

Answer 2

I'll answer the question in your title since nobody else answered it.

According to the docs, lock_guard works on any type that "meets the BasicLockable requirements". BasicLockable only requires two methods, lock() and unlock().

In order to make lock_guard work with a custom library, you need to either add lock() and unlock() methods to the library's mutex class, or wrap it in another class that has lock() and unlock() methods.

Answer 3

You should use those: std::mutex, std::shared_lock, std::unique_lock, std::timed_mutex, std::shared_mutex, std::recursive_mutex, std::shared_timed_mutex, std::recursive_timed_mutex, std::lock_guard if you actually have C++11 compiler. Otherwise it is more complex, and tag c++11 on question should be removed.

You can't implement spinlock or mutex with C\C++ means, you would need add assembler or intrinsic code - making it non-portable, unless you implement it for each platform - and with many x86-64 C++11 and later compilers you can't do inline assembler.

The main problem you would run into, if you use inherent locking logic is that objects behind mutexes or spinlocks are uncopyable. As soon as you copy it or if you copy defended variable, it stops being locked. Objects that are implementing mutex mechanics are uncopyable too.