Re: A scoped lock/unlock implementation in C++.
- From: Marcel Müller <news.5.maazl@xxxxxxxxxxxxxxx>
- Date: Thu, 01 Jan 2009 03:48:19 +0100
Hi!
JC wrote:
Thanks for your reply.
Actually, the mutex_locker (in my implementation) is only intended to
be used by a single thread (although more than one thread may have a
mutex_locker wrapping the same mutex, more than one thread won't share
a mutex_locker). The counter / tracking info only tracks locking
information for that one thread.
In this case at least the assertions of my implementation will not hit the nail on the head. But beyond that it will still work with thread local mutex_locker instances, since no two threads can have a nonzero lock count anyway.
If you really ensure that a mutex_locker instance is not used by more than one thread, you may remove the TID field and all access to it from my code. It will still work. But then you MUST not use it with different threads. This is risky because you cannot easily make the objects thread-local in a way that the compiler detects incorrect useage.
The reason the implementation is so complicated is because it provides
more than just recursive support. It provides one extra feature,
namely a guarantee that a scoped_unlock *will* unlock the mutex lock
even if it is nested inside multiple scoped_locks:
mutex_locker locker(g_mutex);
{
scoped_lock lock(locker); // <-- lock acquired
{
scoped_lock lock(locker); // <-- no-op
{
scoped_unlock unlock(locker); // <-- lock released
// mutex is UNLOCKED at this point.
} // <-- lock acquired
} // <-- no-op
} // <-- lock released
This is exactly what happens with my implementation too.
It provides the same guarantee for scoped_locks nested in multiple
scoped_unlocks. A pure count-based implementation would require a
scoped_unlock to match each scoped_lock before the mutex was actually
unlocked, e.g., with your example:
mutex_locker locker(g_mutex);
{
scoped_lock lock(locker); // <-- acquired (++count)
{
scoped_lock lock(locker); // <-- no-op (++count)
{
scoped_unlock unlock(locker); // <-- no-op (--count)
// <-- mutex is still LOCKED!
No. Count will be decremented by 2 in this case and the mutex is released.
{
scoped_unlock unlock(locker); // <-- released (--count==0)
This would be a no-op.
// <-- now mutex is unlocked.
} // <-- acquired (++count)
no-op
} // <-- no-op (++count)
aquire, count += 2
} // <-- no-op (--count)
} // <-- released (--count==0)
The extra complexity with the vector provides this functionality by
tracking how many levels into similar "regions" you are nested, and
then acquiring/releasing the lock when you transition between locked
and unlocked regions regardless of the nesting "balance".
There is a comment in the source that describes this, but basically
each element of the vector is a count of consecutive entrances to a
given region type (locked or unlocked), with positive integers
representing a locked region and negative integers representing
unlocked. So the vector:
3 -2 4 -1
Means that at the current position, you have locked the mutex 3 times,
then unlocked it 2 times, then locked it 4 times, then unlocked it 1
time (via the mutex_locker), and the mutex is currently *unlocked*.
Note that the vector always starts with a positive number, alternates
signedness, and never contains a 0 (these are invariants of
mutex_locker member functions).
When entering a locked region, if the last entry is negative, you know
you are transitioning from unlocked -> locked and must acquire the
mutex. A new entry, 1, is added. If the last entry is positive, it is
simply incremented but no action is taken on the mutex. The same is
true for entering an unlocked region, but with opposite signs.
Feel free to do so, but I think the complexity is not required to satisfy your needs.
So, the complexity over a simple count-based implementation is
necessary because of the nesting guarantees, it's more than just a
recursive wrapper around non-recursive mutexes.
I don't think so. The complete information in the vector is not needed together at any place in your code. Only the deepmost element counts. So why not distribute the information over the objects in the callstack. This is exactly what I have done. The mutex_locker only keeps track of the last state. In case of unlocks even the unlock count is insignificant.
Furthermore I have the following recommendations:
- Have a look at conditional variables, if they fulfill your needs in a way that you do not have to deal with explicit unlocks at all. This is a cleaner application design in my opinion. (They operate similar to my implementation.) If conditinal variables are no choice then
- implement the core logic to deal with the unlocking in your mutex class and throw away the thread local mutex_locker. This will make things easier, because you never can accidentially operate on a mutex_locker of a different thread.
- The mutex class should always have a public function the check the locked state for the current thread. While this might not be needed in the main code lines, it is very helpful for assertions at the start of function bodies to verify the preconditions 'mutex must be locked' or 'mutex must be unlocked'.
- Do not use unscoped locks or unlocks at all, unless you really want to ensure to have some bugs in your application. Times have changed with C++ and it is nearly impossible to get the unscoped locks exception safe on a sustained basis.
You may protect the mutex::lock and unlock functions and declare the scoped_lock/unlock classes as friends of mutex instead. This ensures a high degree of code consistency.
- If you really want to be able to lock or unlock the mutex at a certain code location, extend the scoped_lock class by an explicit unlock and lock function and a locked flag. Example:
class scoped_lock
{private:
mutex_locker& L;
bool Own;
public:
scoped_lock(mutex_locker& l) : L(l), Own(true) { L.lock(); }
~scoped_lock() { if (Own) L.unlock(); }
unlock() { assert(Own); L.unlock(); Own = false; }
lock() { assert(!Own); L.lock(); Own = true; }
};
Than you will be able to release a lock earlier e.g. in case of an error exit, that should not write the log message to the file system within locked context. However, if you have a nested lock, that won't help you and even a scoped_unlock will not be safe, since the caller cannot be sure about the state of the variables protected the lock when the callee unexpectedly released the lock in case of an error.
- Strictly speaking, once you need a recursive lock, your code is not that clean. It is because this breaks the assumption of a callee, that outside the locked region safely other things can be done. This sometimes results in deadlocks, when function calls outside the locked region might imply other mutex locks, which are no longer deadlock-safe if the callee is called inside a nested lock. Believe, I made the experience - the hard way.
Marcel
.
- Follow-Ups:
- Prev by Date: Re: A scoped lock/unlock implementation in C++.
- Next by Date: Re: Why are Boost thread mutexes so slow compared to Pthreads?
- Previous by thread: Re: A scoped lock/unlock implementation in C++.
- Next by thread: Re: A scoped lock/unlock implementation in C++.
- Index(es):
Relevant Pages
|
