Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Scoped_lock with repeating arguments

I use std::scoped_lock to guard pairs of objects in multi-threading environment. But I found that scoped_lock can lead to deadlock (in Visual Studio and gcc) if both its arguments are the same. For example,

#include <mutex>

struct S
{
    mutable std::mutex m;
    int v = 0;
    S & operator = ( const S & b )
    {
        std::scoped_lock l( m, b.m );
        v = b.v;
        return * this;
    }
};

int main()
{
    S a;
    a = a; //deadlock here!
}

I see that the standard requires “The behavior is undefined if one of MutexTypes is not a recursive mutex and the current thread already owns the corresponding argument in ...”, see https://en.cppreference.com/w/cpp/thread/scoped_lock/scoped_lock

But formally in my example the mutex is not locked before scoped_locked. So is it expected program behaviour?

like image 997
Fedor Avatar asked Nov 07 '22 01:11

Fedor


1 Answers

The phenomena you would really afraid of is "self deadlock". It occurs, when 2 conditions are fulfilled simultaneously:

  • the same thread calls repeatedly mutex.lock() on the same mutex object,
  • and that mutex belongs to non-recursive type, which does not support such recursive locking.

In C++ you have 2 types of mutexes:

  • std::mutex is non-recursive type - it is quicker and requires less resources,
  • std::recursive_mutex is recursive type - it is safer but requires more resources.

Now let's apply this knowledge to your specific example:

  1. In your case, the obvious way to avoid "self deadlock" is to avoid the "self assignment". For this purpose simply add the additional check:

     S & operator = ( const S & b ) 
     {
         if (this != &b) // check it is not self assignment
         {
             std::scoped_lock l( m, b.m );
             v = b.v;
         }
         return * this;
     }
    
  2. If this is the only usage of your mutex, you can be sure that recursive "self deadlocking" will never occur. So, this is the best and cheapest solution.

  3. If you are going to add other "synchronized" method which calls the current one when mutex is already held by calling thread, then (and only in such case) you really need to replace the std::mutex type to be std::recursive_mutex.

like image 139
Daniel Z. Avatar answered Nov 15 '22 05:11

Daniel Z.