Protecting Coders From Ourselves: Better Mutex Protection
At some point, if you’ve done multithreaded programming, you’ve probably used a mutex (or some other locking mechanism). Locks are relatively straightforward to understand and use (“lock this thing before accessing your data and unlock it when you’re done”), but they do have their issues. The most commonly-discussed issue with using locks is the dreaded deadlock, scourge of many a poor soul who needed to hold multiple locks at once for something.
But we’re not here to talk about deadlocks…instead, I’d like to focus on a problem that I have encountered much more frequently: accidentally reading or modifying protected data without having acquired the lock.
That’s right, we’re once again going to try to protect ourselves from our worst enemy: ourselves.
NoteAs with the previous entry in this semi-series, the examples and implementation are in C++, but you can probably build something similar in your language of choice (unless it doesn’t need it).
An Easy Mistake
It turns out it’s surprisingly easy to not grab a lock before reading or writing things that need to be synchronized. Here’s a toy example:
void Foo::IncrementThing(int c)
{
// Lock the mutex correctly!
std::lock_guard lock { m_mutex };
m_thing += c;
}
void Foo::DecrementThing(int c)
{
m_thing -= c; // Whoops, no lock!
}In the real world, “oops I accessed a thing outside of the lock” bugs tend to be more subtle - it can be especially hard to notice when a thing that should be present isn’t. Since all of the accesses are to normal members of your class/struct/global scope/whatever, it’s easy to slip and put an access to a value where it wasn’t intended, and wasn’t propertly protected.
But what if instead you could do something more like the following:
// Note: this is pseudocode, not actual C++
class Foo
{
// ... class stuff ...
// Mysterious m_state object that protects everything inside of it with a mutex
MutexProtected m_state
{
int m_thing = 0;
};
};
void Foo::IncrementThing(int c)
{
lock (m_state) // Lock here
{
m_thing += c; // Only accessible in the lock
}
// m_thing += c; // Won't compile: Can't access outside of lock
}
Basically, what if you could have some sort of protected wrapper around the state that walls it off and makes it inaccessible except when the lock is actually held? With something like that it would become much more difficult to get at values when it shouldn’t be allowed.
Additionally, it would help both with organization (forcing the mutex-protected values together in the code) as well as making the intent clear: values that are protected sit inside the protected block, clarifying which values are (and are not) intended to be accessed solely through the mutex, even to folks who were not the original author (the list of which stealthily includes the original author, one month in the future).
With C++ it’s possible to get quite close to the above:
class Foo
{
// ... class stuff ...
// State structure for the protected member(s)
struct State { int m_thing = 0; };
// The mutex and the state it's protecting
MutexProtected<State> m_state;
};
void Foo::IncrementThing(int c)
{
// lock the mutex and get the inner state
auto state = m_state.Lock();
// Use the lock to access the values within.
state->m_thing += c;
}The nice thing is, a basic implementation of this is fairly compact. There are just two classes involved: MutexProtected<T> and its lock object, MutexLocked<T>.
EDIT (2025-01-25): People have pointed out two existing versions of this: boost has
boost::synchronized_value, and there’s cs_libguarded which looks like it has a few variants on this concept, too. So if you don’t feel like rolling your own you can always check one of those out instead!
MutexProtected<T>
The outer class, MutexProtected<T>, has very few moving parts:
template <typename T>
class MutexProtected
{
public:
MutexLocked<T> Lock()
{ return { &m_t, m_mutex }; }
private:
std::mutex m_mutex;
T m_t;
};There’s a lock function that returns a MutexLocked<T> (which we’ll get to in a moment), and it contains both a mutex as well as a T (the templated type), which contains the data to be protected by the mutex. In the earlier example, this was the State struct that contained the protected m_thing value, but it could contain any number of values/objects that should only be accessed from within the same lock.
NoteYou may be wondering “there’s a
Lock, so why is there no correspondingUnlockfunction?” - this is because we’re taking advantage of the classic C++ idiom of RAII, so the returnedMutexLocked<T>object holds the lifetime of the lock, and unlocks the mutex when it goes out of scope. As such, there’s no need to manually unlock the mutex; it will happen automatically.
MutexLocked<T>
But what is the thing that the Lock function returns? Well, that’s the MutexLocked<T> class and it looks like this:
template <typename T>
class MutexLocked
{
public:
// Add the standard pointer-like accessors.
T *operator->() const { return m_p; }
T &operator *() const { return *m_p; }
private:
// Construct this with a pointer to the state and the mutex to lock.
MutexLocked(T *p, std::mutex &mutex)
: m_lock{std::lock_guard(mutex)}, m_p{p}
{ }
std::lock_guard<std::mutex> m_lock;
T *m_p = nullptr;
template <typename T>
friend class MutexProtected;
};As you can see, it also doesn’t have much to it! It constructs with a pointer to the state object (the m_t in the MutexProtected<T>) and the mutex (which it immediately locks in the constructor), and so all it does is hold onto the lock until destruction time, while giving the user a way to access the state object (via the two public operators).
So, using this setup in full:
- Call
Lockon theMutexProtected<T>to get aMutexLocked<T> - Access the protected state through that acquired object (using the
->operator) to do whatever needs to be done within the lock - Let the
MutexLocked<T>leave scope, at which point the lock is released
A nice little feature of this is if you do have a single thing that you’re doing in the lock (Say, inserting a value into a protected queue), the above steps can even fit nicely on a single line (while still being perfectly readable), thanks to the rules of C++ temporary object lifetimes:
void Foo::EnqueueItem(Item *i)
{
state.Lock()->queue.Enqueue(i); // Lock/Update/Unlock
// Do more stuff here outside of the lock, it's guaranteed to be released
}Sub-Functions That Require A Lock
This type of object also helps with a secondary part of this problem, which is when your class has functions (that are likely private or protected) that require the lock to already be acquired before you call it:
// This function is intended to only be called while the lock is held
void Foo::AdjustStateWithLockHeld(int delta)
{ m_thing += delta; }
Void Foo::IncrementThing(int delta)
{
std::lock_guard lock { m_mutex };
// Call this function while the lock is held only
AdjustStateWithLockHeld(delta);
}
Void Foo::DecrementThing(int delta)
{
// Oh no I have once again forgotten to lock the mutex before doing the thing
AdjustStateWithLockHeld(-delta);
}In the above example, AdjustStateWithLockHeld is assuming that the lock is being held by its caller and that it’s free to manipulate the state within it. It doesn’t make much sense in this example, but if you have mutex-protected state that has a complex update process (such as multiple things needing to be kept in sync), it can be nice to move such logic to a subroutine.
Thankfully, using the MutexProtected<T> object, the state is only accessible through a MutexLocked<T> object. In order for the sub-function to be able to do anything with the state, it would need to additionally take a reference to the MutexLocked<T> for the state in question, thus effectively ensuring that the mutex is locked by the caller:
// Now the function takes the locked State object as a parameter:
void Foo::AdjustStateWithLockHeld(
MutexLocked<State> &state,
int delta)
{ state->m_thing += delta; }
Void Foo::DecrementThing(int delta)
{
// Now state has to be locked to get the object to pass to the sub-function!
auto state = m_state.Lock();
AdjustStateWithLockHeld(state, -delta);
}A Variation
For completeness, I also wanted to mention an alternate form of this that I thought of while designing it: where, rather than Lock returning an object that represents the scoped lock, you instead pass a lambda (or function) that gets all of the state values in it:
struct State
{
int a, b;
};
MutexProtected<State> state;
state.Lock(
[&](int &a, int &b)
{
// "a" and "b" correspond to the two values of the same name in the state structure.
// All the things that need to modify state values must, then, happen in this lambda,
// as there is no way to access the struct members directly.
a += someVariable;
b -= 2;
});The main advantage this has over the other form is that it makes it considerably more difficult to accidentally (or intentionally?) grab a reference to the internal state structure that could then persist outside of the lock. However, in practice I felt like that kind of mistake is not going to be super common relative to the problem being solved, but also should be easy to catch during code review. Plus, there are additional downsides to this version that I didn’t like:
- It’s easy to get the order or names of the lambda paramters wrong and end up doing the wrong things with the wrong values since they’d effectively have to match the order in the state.
- It gets harder to call sub-functions that need the lock (you’d have to pass all of the state objects that need updating which can be a pain in practice).
- It’s worse to debug, since you have to step into the Lock call instead of just over it, and then into the lambda from there.
- This is also the reason I didn’t consider another variant where instead of a parameter per state object it’s a single reference to
Stateand you access it that way.
- This is also the reason I didn’t consider another variant where instead of a parameter per state object it’s a single reference to
All said, I felt like having a lock object that provides access to the inner state as-is (via the -> operator) was cleaner in practice, and easier to step through in the debugger.
Limitations
This, of course, isn’t a perfect solution:
- There are absolutely cases where some things need to be accessible outside of the mutex lock (i.e. an atomic which can be safely read at any time but only gets updated from within the mutex due to sequencing issues), and as such those values couldn’t live inside of the inner state object.
- Also, this is C++ so there’s nothing preventing someone from grabbing a reference or pointer to the state object from the lock and holding onto it until after the lock ends, then partying on the data. However, that kind of code is more likely to get caught at review time.
- There are likely other cases (multiple locks, perhaps, which I haven’t fully thought through with this because I haven’t needed to) where this would present problems. This is definitely primarily intended for the “I have a set of data that should only ever be accessed during a lock” case.
Potential Improvements
Also, there are some improvements that could be made:
- Perhaps instead of using a
lock_guardyou could use aunique_lockwhich would let you wait on a critical section using the lock (if you need to wait for the state to be in some specific configuration), which could even be built into theMutexProtected<T>class (or a similar one) if the critical section is a core part of the usage. - It’s a good idea to declare a custom move constructor and move assignment operator for
MutexLocked<T>that nulls them_tpointer of the object being moved from so that you can’t do a move of it to some other location (which subsequently releases the lock) and then still party on the internal pointer. (I also have asserts in my production version in the pointer-access operators that assert the pointer is non-null) - Similarly, it may be nice to have a constructor for
MutexProtected<T>that takes constructor arguments for the contained state structure (similar to, saystd::vector::emplace_back), especially if you are going to have state structures that cannot default construct. - There are also other flavors of locking that might occur: for instance, a
TryLockfunction that returns astd::optional<MutexLocked<T>>, and only locks the lock (and returns the locked state) if there is no contention on the mutex.
Closing Time
All in all, having an abstraction like this makes it way more difficult to party all over internal state without properly locking the mutex first. Switching some old code to use this actually found a couple places where I’d done things incorrectly (reading values that should have been mutex protected on read, in those cases).
So, yeah, by protecting our data from being accessed when it shouldn’t be, we’re also protecting ourselves from ourselves.