Technical Writings
- C++ Concurrency Book
- Articles and Publications
- Conference Presentations
- Recent Blog Entries
- Multithreading Blog Entries
- C++ Blog Entries
Subscribe to Blog
Blog Archives
Simplify Code by Encapsulating Locks
Wednesday, 15 June 2011
Over on the Future Chips blog, Aater Suleman argues
that while(1)
can make parallel code better. Whilst I agree that the code
using while(1) is simpler than the original in terms of
analysing the lock patterns, it achieves this by testing the logical
condition inside the while and
using break. This is additional, unnecessary,
complexity.
What is wanted instead is a way of encapsulating the locking, so that the loop logic is simple, and yet the lock logic is also clear.
Here is the original code from the blog post:
lock_acquire(lock);
while(check_condition()){
lock_release(lock);
//do any actual work in the iteration - Thanks to Caleb for this comment
lock_acquire(lock);
}
lock_release(lock);
The implication here is that check_condition() must be
called with the lock held, but the lock need not be held for the
actual iteration work. The code thus acquires and releases the mutex
in two places, which is unnecessary duplication, and a potential
source of errors — if the loop exits early then the lock may
be released twice, for example.
Rather than moving the condition check into the loop to avoid this duplication, a better solution is to move the lock acquisition and release into the condition check:
bool atomic_check_condition()
{
lock_acquire(lock);
bool result=check_condition();
lock_release(lock);
return result;
}
while(atomic_check_condition()){
//do any actual work in the iteration - Thanks to Caleb for this comment
}
This gives us the best of both worlds: the lock is now held only
across the check_condition() call, but the logic of
the while loop is still clear.
If you're programming in C++, then the C++0x library allows us to
make atomic_check_condition() even simpler by
using lock_guard as in the code below, but extracting
the function is always an improvement.
bool atomic_check_condition()
{
std::lock_guard<mutex_type> guard(lock);
return check_condition();
}
Posted by Anthony Williams
[/ threading /] permanent link
Tags: multithreading, concurrency, locks
Stumble It! 
| Submit to Reddit 
| Submit to DZone 
If you liked this post, why not subscribe to the RSS feed 
or Follow me on Twitter? You can also subscribe to this blog by email using the form on the left.
Design and Content Copyright © 2005-2024 Just Software Solutions Ltd. All rights reserved. | Privacy Policy
8 Comments
This is the usual process followed by all to have the gems and gold for the game that is so interesting to play.
Hi Anthony,
It is also possible to use for instead. Actually for is more powerful, so you can do something like that.
for (std::unique_lock<std::mutex> lk(m, std::defer_lock); lk.lock(), check_condition(); lk.unlock()) { //do any actual work in the iteration }@TA: Your for statement has different semantics: the lock is only released at the end of each iteration, not immediately after checking the condition.
Even if the semantics were the same, I would still prefer encapsulating the lock in atomic_check_condition() --- it's clearer all round.
Yes, that's right. lk.unlock() should be moved to the body, however making a function allows code reuse, so in most of cases I think that is more preferable solution.
I must be listening to Herb Sutter too much... This looks to me another place where lambdas can be used (assuming <code>atomic_check_condition</code> isn't used anywhere else.) <pre> while(([]() -> bool { std::lock_guard<mutex_type> guard(lock); return check_condition(); })()){ //do any actual work in the iteration } </pre>
@Motti: Yes, you could use a lambda, but I think a separate function would be clearer --- the name clearly specifies what it is doing, and reduces the clutter. On the other hand, a named lambda might be useful:
auto atomic_check_condition=[&](){std::lock_guard<mutex_type> guard(lock); return check_condition();};
while(atomic_check_condition()) { ... }
On one project I wrote a custom locking smart pointer whose operator->() returned a helper class whose constructor locked and destructor unlocked before and after its own operator->() was called. This meant that I could write code like this:
while (! server->stopped) { .... }
where server is an instance of said locking smart pointer. The code has the advantage of being clear and you cannot forget to lock or unlock (presuming that you can't access the underlying data directly). I was using the Poco libraries so this code uses a Poco::Mutex. The CONTENTION() macro is non-empty for testing purposes: it tries to lock the mutex and records in a file whether it succeeded or not along with __FILE__ and __LINE__. This allows me to see if the mutex is a contention hotspot or not (it wasn't, as I suspected).
namespace utils { template <typename T> class LockingPtr { T * ptr; Poco::Mutex mutex; public: class LockingPtrHelper { LockingPtr * parent; public: T * operator->() { return parent->ptr; }
explicit LockingPtrHelper(LockingPtr * p) : parent(p) { CONTENTION(parent->mutex).lock(); }
~LockingPtrHelper() { parent->mutex.unlock(); } };
friend class LockingPtrHelper; explicit LockingPtr(T * p = 0) : ptr(p) {} ~LockingPtr() { delete ptr; }
// Stop copying explicit LockingPtr(const LockingPtr & other); LockingPtr operator=(const LockingPtr & rhs);
LockingPtrHelper operator->() { return LockingPtrHelper(this); }
/// Swap pointers over. Use addresses of mutexes to enforce /// a global ordering on locking to avoid deadlocks. void swap(LockingPtr & other) { if (&mutex < &other.mutex) { CONTENTION(mutex).lock(); CONTENTION(other.mutex).lock(); } else { CONTENTION(other.mutex).lock(); CONTENTION(mutex).lock(); } std::swap(ptr, other.ptr);
if (&mutex < &other.mutex) { mutex.unlock(); other.mutex.unlock(); } else { other.mutex.unlock(); mutex.unlock(); } } }; }@Hubert: Yes, wrapping the mutex lock in a type like this is the next logical step after encapsulating it in a function.
An alternative to a locking pointer is to wrap the mutex and data together, so all accesses to the data lock the mutex. See my article at DDJ: http://drdobbs.com/cpp/225200269