Mutexes and deadlocks
After asking about mutexes here, I was warned about deadlocks.
Could the example below be a sane way to avoid deadlocks?
class Foo
{
public:
Foo();
void Thread();
int GetWidgetProperty();
int GetGadgetProperty();
private:
Widget widget_;
Gadget gadget_;
VDK::MutexID widgetLock;
VDK::MutexID gadgetLock;
};
Foo::Foo()
: widget_(42)
, gadget_(widget_)
{
widgetLock = VDK::CreateMutex();
gadgetLock = VDK::CreateMutex();
}
void Foo::Thread()
{
while(1)
{
VDK::AcquireMutex(widgetLock);
// Use widget
VDK::ReleaseMutex(widgetLock);
VDK::AcquireMutex(widgetLock);
VDK::AcquireMutex(gadgetLock);
// use gadget
VDK::ReleaseMutex(widgetLock);
VDK::ReleaseMutex(gadgetLock);
}
}
int Foo::GetWidgetProperty()
{
VDK::AcquireMutex(widgetLock);
return widget_.GetProp();
VDK::ReleaseMutex(widgetLock);
}
int Foo::GetGadgetProperty()
{
VDK::AcquireMutex(widgetLock);
VDK::AcquireMutex(gadgetLock);
return gadget.GetProp();
VDK::ReleaseMutex(widgetLock);
VDK::ReleaseMutex(gadgetLock);
}
Since the call to GetGadgetProperty might result in the use of the widget, I guess we also need to protect ourselves with a lock here. My question is, do I claim and release them in the correct order?
source to share
There is an obvious dead end in your code. You cannot free a mutex after return expression. Moreover, it is better to unlock them in reverse order (to lock). The correct code should look like this:
int Foo::GetWidgetProperty()
{
VDK::AcquireMutex(widgetLock);
int ret = widget_.GetProp();
VDK::ReleaseMutex(widgetLock);
return ret;
}
int Foo::GetGadgetProperty()
{
VDK::AcquireMutex(widgetLock);
VDK::AcquireMutex(gadgetLock);
int ret = gadget.GetProp();
VDK::ReleaseMutex(gadgetLock);
VDK::ReleaseMutex(widgetLock);
return ret;
}
source to share
An even better way is to rely on RAII to get the job done for you.
I invite you to read std :: lock_guard . The basic principle is that you acquire a mutex by declaring an object. And the mutex is automatically freed when it expires.
Now you can use block scopes for a code region that should block mutexes like this:
{
std::lock_guard lockWidget{widgetMutex};//Acquire widget mutex
std::lock_guard lockGadget{gadgetMutex};//Acquire gadget mutex
//do stuff with widget/gadget
//...
//As the lock_guards go out of scope in the reverse order of
//declaration, the mutexes are released
}
Of course this works with the standard library mutexes, so you'll have to adapt to your use.
This would prevent an error, such as trying to free the mutex after a return statement that obviously never happens, or in the face of an exception that happens before the mutex is actually released.
source to share