After reading Herb Sutter's Associate Mutexes with Data to Prevent Races, I found that my solution was superior in several aspects, least important first:
- The code is cleaner, without macros
- No code added to every struct/class
- Does not prevent the use of non-protected versions of the classes/structs
- Does not rely on code coverage, which is subject to omission (relies on the compiler instead)
The idea is to never give access to an object which associated mutex has not been locked. The locking is still explicit, but without it, one simply can't do anything with the objects. One passes around Lockable. Lockable has a mutex and a T. To be able to access T, one has to call Lockable::GetLockedProxy() that returns a LockedProxy object. A LockedProxy has a scoped_lock and a T*, and allows operations on the T.
To access the T, one uses LockedProxy::operator->() or operator*().
While a LockedProxy is alive, that is, while one has access to the T, the associated scoped_lock is alive, meaning that the mutex is locked, and subsequent calls to GetLockedProxy, which will try to create a scoped_lock, will hang.
The drawback is, one still can be nasty and Lock the object and get a pointer to it (via LockedProxy's operator*) and store it, then release the lock and use the pointer. That's the abstraction's leak. But that's pointing the gun to one's foot and pulling the trigger.
The classes: Lockable.h
#include <boost/thread/mutex.hpp>
#include <boost/interprocess/sync/scoped_lock.hpp>
template<typename T>
class LockedProxy : boost::noncopyable
{
public:
inline LockedProxy(boost::mutex & m, T * obj)
:lock(m),
t(obj)
{}
inline LockedProxy(LockedProxy && other)
:lock(std::move(other.lock)),
t(std::move(other.t))
{}
inline T * operator->() { return t; }
inline const T * operator->() const { return t; }
inline const T & operator*() const { return *t; }
inline T & operator*() { return *t; }
private:
boost::interprocess::scoped_lock<boost::mutex> lock;
T * t;
};
template<typename T>
class Lockable
{
public:
// Convenience typefed for subclasses to use
typedef T LockableObjectType;
inline Lockable(const T & t)
:lockableObject(t)
{}
inline LockedProxy<LockableObjectType> GetLockedProxy() {
return LockedProxy<LockableObjectType>(mutex, &lockableObject);
}
protected:
LockableObjectType lockableObject;
boost::mutex mutex;
};
How to use them:
#include <iostream>
#include <string.h>
#include "Lockable.h"
void f(Lockable<std::string> & str)
{
auto proxy = str.GetLockedProxy();
*proxy = "aa";
proxy->append("bb");
std::cout << "str = " << *proxy << std::endl;
}
void g(Lockable<int> & i)
{
{ // reduce lock's lifespan
auto proxy = i.GetLockedProxy();
*proxy = 321;
}
// relock, lock lives for the statement
std::cout << "i = " << *i.GetLockedProxy() << std::endl;
}
int main()
{
Lockable<std::string> str("abc");
//Can't use str here, it is not locked
f(str);
Lockable<int> i(123);
g(i);
return 0;
}
The usual question goes here: What do you think ?
Are there drawbacks or pitfalls I did not see ?
Do other libs like boost have something similar that I should use instead ?
EDIT:
The following code implements @useless', @Loki Astari's and my own suggestions and should compile with both VS and g++.
I renamed Lockable
and LockedProxy
into Synchronized
and SynchronizedProxy
, for I found the former could lead to misunderstanding.
The classes have gronw some hair, but their use have not, which is what matters. Using proxy
after TryGetSynchronizedProxy
failed is equivalent to using a nullptr
which throws, which is good.
Getting rid of the dependency on boost was a bit tricky. TryLock has some caveats in C++ 11 as described here, which motivates the use of recursive_mutex
as default.
boost' safe bool is no longer required thanks to this, but it does not compile with VS2012, even with the november 2012 compiler update.
Synchronized.hpp:
#include <mutex>
template<typename T, class Mutex>
class SynchronizedProxy
{
template<typename X, class Y>
friend class Synchronized;
private:
SynchronizedProxy();
SynchronizedProxy(const SynchronizedProxy &);
SynchronizedProxy & operator=(const SynchronizedProxy &);
SynchronizedProxy & operator=(SynchronizedProxy &&);
SynchronizedProxy(Mutex & m, T & obj)
:lock(m)
,t(&obj)
{}
SynchronizedProxy(Mutex & m, T & obj, int)
:lock(m, std::try_to_lock)
,t((lock)?&obj:nullptr)
{}
public:
SynchronizedProxy(SynchronizedProxy && other)
:lock(*other.lock.mutex(), std::adopt_lock)
,t(std::move(other.t))
{
other.t = nullptr;
}
explicit operator bool() const { return (bool)lock; }
const T * operator->() const { return t; }
T * operator->() { return t; }
const T & operator*() const { return *t; }
T & operator*() { return *t; }
private:
std::unique_lock<Mutex> lock;
T * t;
};
template<typename T, class Mutex=std::recursive_mutex>
class Synchronized
{
public:
// Convenience typefed for subclasses to use
typedef T SynchronizedObject;
Synchronized(const T & t)
:t(t)
{}
SynchronizedProxy<T,Mutex> GetSynchronizedProxy() {
return SynchronizedProxy<T,Mutex>(mutex, t);
}
SynchronizedProxy<T,Mutex> TryGetSynchronizedProxy() {
return SynchronizedProxy<T,Mutex>(mutex, t, 0);
}
protected:
T t;
Mutex mutex;
};
main.cpp:
#include <iostream>
#include <thread>
#include <string>
#include "Synchronized.hpp"
void f(Synchronized<std::string> & str)
{
auto proxy = str.GetSynchronizedProxy();
*proxy = "aa";
proxy->append("bb");
std::cout << "str = " << *proxy << std::endl;
}
void g(Synchronized<int> & i)
{
{ // reduce lock's lifespan
auto proxy = i.GetSynchronizedProxy();
*proxy = 321;
}
// relock, lock lives for the statement
std::cout << "i = " << *i.GetSynchronizedProxy() << std::endl;
}
int main()
{
Synchronized<std::string> str("abc");
//Can't use str here, it is not locked
f(str);
Synchronized<int> i(123);
g(i);
{
auto proxy = i.TryGetSynchronizedProxy();
if (proxy)
{
*proxy = 222;
}
}
{
auto p = i.GetSynchronizedProxy();
std::thread([&i]
{
auto proxy = i.TryGetSynchronizedProxy();
if (proxy)
{
*proxy = 333;
}
}
).join();
}
std::cout << "i = " << *i.GetSynchronizedProxy() << std::endl;
return 0;
}
Output:
str = aabb
i = 321
i = 222