Careful with std::mutex

 recursive_mutex

 std::recursive_mutex can be locked again by the owning thread without blocking. A normal std::mutex doesn't support this behavior and attempt to lock it again from the owning thread is even undefined behavior.

 Still C++ experts promote std::mutex over std::recursive_mutex. The locking is more clear but programmers need to take extra care now.

Example

 Suppose a class with two data members (A and B) and the data needs to be protected from access by multiple threads. The class has two functions; one to change A and one to change A and B. The following implementation is wrong:

#include <mutex>

class Dummy
{
public:
   void SetA(int a)
   {
      std::unique_lock<std::mutex> lck(m_mtx);

      m_a = a;
   }

   void SetAB(int a, int b)
   {
      std::unique_lock<std::mutex> lck(m_mtx);

      m_b = b;

      SetA(a);  // error: mutex is still locked
   }

private:
   std::mutex  m_mtx;
   int         m_a;
   int         m_b;
};

  In the function SetAB the mutex is still locked when invoking SetA leading to undefined behavior.

 Fixing this by adding a SetB function and invoking this function is also incorrect:

class Dummy
{
public:
   void SetA(int a)
   {
      std::unique_lock<std::mutex> lck(m_mtx);

      m_a = a;
   }

   void SetB(int b)
   {
      std::unique_lock<std::mutex> lck(m_mtx);

      m_b = b;
   }

   void SetAB(int a, int b)
   {
      SetA(a);  
      SetB(b);  
   }

   // rest same as before
};

  This is wrong because of another reason: A and B are now not modified under the same lock. A race condition may emerge where A is already changed but B not yet. This may break an invariant if A and B need to be updated together.

 There are multiple solutions to this issue:

• differentiate between locking and non locking (implementation) functions. Drawback is that the number of (private) member functions are increasing and thereby obfuscating the design
• set member data A and B directly without using member or other functions. For simple functions like class above this is okay but many times the SetXxx member function does extra things (e.g. notifying clients). Duplicating these code is not attractive.
  

 Example of differentiate between locking and non locking functions:

class Dummy
{
public:
   void SetA(int a)
   {
      std::unique_lock<std::mutex> lck(m_mtx);

      SetAImpl(a);
   }

   void SetB(int b)
   {
      std::unique_lock<std::mutex> lck(m_mtx);

      SetBImpl(b);
   }

   void SetAB(int a, int b)
   {
      std::unique_lock<std::mutex> lck(m_mtx);

      SetAImpl(a);
      SetBImpl(b);
   }

private:
   void SetAImpl(int a)
   {
      m_a = a;
   }

   //etc.
   
   // rest same as before
};

  Above example is a simplified version. In real code classes may have dozens of member functions. Also the invocation of another member function when the mutex is already locked may happen indirect; e.g. through an event notification.