patterncppMinor
Force locking for thread-safety
Viewed 0 times
forcethreadforsafetylocking
Problem
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 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
#include
template
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 lock;
T * t;
- 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
#include
template
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 lock;
T * t;
Solution
Since the only place you should be creating a
Also since the access to this constructor is so controlled you never really want to pass a NULL object so you should pass
You should not be using the keyword
Does it make sense to be able to move the Locked Proxy?
Not sure. I don't understand the use case where you would want or need to do that.
Auto conversion to bool?
Why? Again I don't see the use case. Also you should probably look up the safe bool idiom. The problem with bool is that is auto converted to integer so the compiler will now be able to compile your code what it see this:
Rather than pass the object in. You should look up variadic templates.
This will allow your constructor here to take the same arguments as the T and forward them directly to the T object being constructed. So now the only T is the one inside the object.
Same comments about inline as above.
LockedProxy from is within the call Lockable::GetLockedProxy() you should therefore make the constructor private and friend this function. You don't actually want to give people the ability to make objects of this type themselves.inline LockedProxy(boost::mutex & m, T * obj)
:lock(m),
t((lock)?obj:nullptr)
{
}Also since the access to this constructor is so controlled you never really want to pass a NULL object so you should pass
obj by reference. If you must store it internally as a pointer fine take the address of the reference parameter for local storage but personally I would maintain everything as references.You should not be using the keyword
inline unless you are required to do so. Declaring and defining it inside the class makes it automatically tagged inline. And apart from linking it has no affect on the compiler. So avoid this keyword unless you actually need in and you don't (it just clutters the code and makes people think you are trying to inline code (which only happens if the compiler thinks it is required)).Does it make sense to be able to move the Locked Proxy?
inline LockedProxy(LockedProxy && other)Not sure. I don't understand the use case where you would want or need to do that.
Auto conversion to bool?
inline operator bool() const
{
return lock;
}Why? Again I don't see the use case. Also you should probably look up the safe bool idiom. The problem with bool is that is auto converted to integer so the compiler will now be able to compile your code what it see this:
LockedProxy& x = /* Get a lock proxy */;
int y = 10 + x; // Will now compile fine (you probably want a compile error here).Rather than pass the object in. You should look up variadic templates.
This will allow your constructor here to take the same arguments as the T and forward them directly to the T object being constructed. So now the only T is the one inside the object.
inline Lockable(const T & t)
:lockableObject(t)
{}Same comments about inline as above.
inline LockedProxy GetLockedProxy() {
return LockedProxy(mutex, &lockableObject);
}Code Snippets
inline LockedProxy(boost::mutex & m, T * obj)
:lock(m),
t((lock)?obj:nullptr)
{
}inline LockedProxy(LockedProxy && other)inline operator bool() const
{
return lock;
}LockedProxy& x = /* Get a lock proxy */;
int y = 10 + x; // Will now compile fine (you probably want a compile error here).inline Lockable(const T & t)
:lockableObject(t)
{}Context
StackExchange Code Review Q#15632, answer score: 8
Revisions (0)
No revisions yet.