1

From this post, I learned the following:

  1. I can inherit from std::mutex, and check if m_holder equals to the current thread id. I think the way is kind of hack, since it is considered a bad idea to inherit from std class. Moreover, m_holder is some implementation of some C++ library which is not a standard interface
  2. I can use owns_lock of unique_lock. However, it is a bad idea, since the method only checks if the current lock holds the mutex. We don't use unique_lock in this way.
  3. A recursive_mutex can save us from deadlock when we double lock a mutex in a thread. However, we must firstly check our code design before we decide that we really need to use this. However, I think I have a good design. What I need is a runtime check that if a certain mutex is locked twice by a thread, it will crash rather than dead lock. So a recursive_mutex seems not suitable for me.
Jan Schultke
  • 17,446
  • 6
  • 47
  • 96
calvin
  • 2,125
  • 2
  • 21
  • 38
  • 1
    I think you mentioned the solution in step 3. Make sure the threading design is sound, based on a locking strategy that is chosen up front (you can't come in after the fact an shoehorn thread safety in, specially not with deadlocks in place). Option4 : if performance/latency isn't your biggest problem : use an executor (dispatcher), that has a queue of tasks (lambdas) and executes functions one by one in the code that needs to be threadsafe. And don't be too spooked by deadloack, you can debug them I prefer them over race conditions. – Pepijn Kramer Jun 15 '23 at 16:33

1 Answers1

1

There are ways to have your cake and eat it with the first solution. We could achieve exactly the same result if we wrapped the std::mutex as a member instead of inheriting from it:

class mutex
{
    std::mutex m_mutex;
    std::atomic<std::thread::id> m_holder = std::thread::id{};

public:
    void lock()
    {
        // TODO: if we wanted to, we could check locked_by_caller()
        //       and panic if true
        m_mutex.lock();
        m_holder = std::this_thread::get_id(); 
    }
    
    bool try_lock()
    {
        if (m_mutex.try_lock()) {
            m_holder = std::this_thread::get_id();
            return true;
        }
        return false;
    }

    void unlock()
    {
        m_mutex.unlock();
    }

    bool locked_by_caller() const
    {
        return m_holder == std::this_thread::get_id();
    }
};

Original code provided by @AmiTavory at https://stackoverflow.com/a/30109512/5740428

Moreover, m_holder is some implementation of some C++ library which is not a standard interface

No, you've misunderstood the original code. m_holder is a member we defined ourselves. We're not relying on any particular standard library implementation. In general, standard library members are named something like _M_holder to use reserved names which can't be replaced by macros.

Jan Schultke
  • 17,446
  • 6
  • 47
  • 96
  • 2
    I think this is a good debugging aid. If OP has multiple mutexes involved in a deadlock the panic code will still not trigger. However it can be used to analyze which threads hold which locks (if the debug build does not already supports this) – Pepijn Kramer Jun 15 '23 at 17:06