Is automating mutex like this in C++ safe?

MiniFalafel 0
Viewed 61

I'm learning about mutex and threading right now. I was wondering if there's anything dangerous or inherently wrong with automating mutex with a class like this:

class AutoMutex
{
private:
    std::mutex& m_Mutex;

public:
    AutoMutex(std::mutex& m) : m_Mutex(m)
    {
        m_Mutex.lock();
    }
    ~AutoMutex()
    {
        m_Mutex.unlock();
    }
};

And then, of course, you would use it like this:

void SomeThreadedFunc()
{
    AutoMutex m(Mutex); // With 'Mutex' being some global mutex.
    // Do stuff
}

The idea is that, on construction of an AutoMutex object, it locks the mutex. Then, when it goes out of scope, the destructor automatically unlocks it.

You could even just put it in scopes if you don't need it for an entire function. Like this:

void SomeFunc()
{
    // Do stuff
    {
        AutoMutex m(Mutex);
        // Do race condition stuff.
    }
    // Do other stuff
}

Is this okay? I don't personally see anything wrong with it, but as I'm not the most experienced, I feel there's something I may be missing.

mutex thread-safety multithreading c++
1 Answers

It's safe to use a RAII wrapper, and in fact safer than using mutex member functions directly, but it's also unnecessary to write since standard library already provides this. It's called std::lock_guard.

However, your implementation isn't entirely safe, because it's copyable, and a copy will attempt to re-unlock the mutex which will lead to undefined behaviour. std::lock_guard resolves this issue by being non-copyable.

There's also std::unique_lock which is very similar, but allows things such as releasing the lock within the lifetime. std::scoped_lock should be used if you need to lock multiple mutexes. Using multiple lock guard may lead to deadlock. std::scoped_lock is also fine to use with a single mutex, so you can replace all uses of lock guard with it.

eerorika0
Related