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;
};

• user673679: Please make sure the posted code works correctly. I'd also suggest adding usage examples, and unit tests.

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.

• Toby Speight: Ah, that's much better than my (removed) suggestion. I couldn't see how to prevent the T& outliving the Access, but I think you've nailed it. We do still have the ability to retain an Access whose T has been destroyed; I don't know whether we can do anything about that.
• darune: I sort of arrived at the same conclusion about the member object access on my own after poking a bit around. Thank you.

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)...}
{
}