Mutex Protected Object (scoped object wrapper)

Question

This is a class that holds mutex and associated object. On checkout/access the mutex is locked while the stack object lives and a reference to object is obtained. Any suggestions or improvements is welcomed.

#include <mutex>

template<class T>
class MutexProtectedObject {
  //this relies on benign use and it is just a helper rather than a foolproof solution - ie. anyone may still save a pointer to obj etc. and access it later
public:
  class Access {
  public:
    Access(std::mutex& m, T& obj) : sl_(m), obj_(obj) {
    }
    T& obj_;
  private:
    std::scoped_lock<std::mutex> sl_;//mutex is locked here (by RAII)
  };

  Access access() {
    return Access(mutex, obj);
  }

  MutexProtectedObject(const MutexProtectedObject&) = delete;
  MutexProtectedObject& opreator=(const MutexProtectedObject&) = delete;

private:
  std::mutex mutex;
  T obj;
};

Answer 1

Access

I don't like the fact that Access gives you a reference to the object directly. This leads to issues with somebody accidentally keeping a reference to the object after the Accesses object is destroyed or accidentally making a copy of the object.

T&  myRef = wrapped.access().obj_;   // Now I have a reference to the object
                                     // But the Accesses object is gone.

myRef.doStuffWithNoLock();


// or

T   myCopy = wrapped.access().obj_;  // I made a copy of the object.

How about Access delegating all accesses via operator ->? That way you don't leak the object to external code.

class Access
{
    public:
        T* operator->() {return &obj_;}
    private:
        T&  obj_;
};

Now you can use Access like it was a T*:

auto foo = wrapper.access();
foo->fun1(args);
foo->fun2(args);

Initializer list

The order of initialization of members is the order of declaration. If you define the order deferently in the initializer list this can be confusing (most compilers will warn you about it).

class Access {
    public:
    Access(std::mutex& m, T& obj)
        : sl_(m)             // This is not the order they are
        , obj_(obj)          // initialized in. The obj_ will be done first
    {}

    T& obj_;

    private:
    std::scoped_lock<std::mutex> sl_;//mutex is locked here (by RAII)
};

Best practice is to keep the initializer list in the same order as the declaration order in the class.

Answer 2

There's no way to construct a MutexProtectedObject unless T is default-constructible. I recommend a forwarding constructor, something like (untested):

template<typename... Args>
MutexProtectedObject(Args&&... args)
  : mutex{},
    obj{std::forward<Args>(args)...}
{
}