c++thread-safetymutex

Safe for an object to return a lock on itself?


Is this code correct / advisable style? That is, is the lock being returned guaranteed to keep the mutex locked through the process of being returned , as opposed to say unlocking then re-locking (or even unlocking and staying unlocked). I am worried about the action of the destructor of the return value.

#include <mutex>

struct BigObject
{
    auto get_lock()
    {
         return std::scoped_lock<std::mutex>(m_mutex);
    }
private:
    std::mutex m_mutex;
};

BigObject big;

int main()
{
    auto lock = big.get_lock();
    
    // ...
}

Assume context of some larger program with concurrent access to the global big.

The "normal" style would be for the caller to declare and initialize the lock, but here we encapsulate the mutex and have the caller call a function. So the object can only be locked via this entry point.

Bonus question - would std::lock_guard instead of std::scoped_lock also be correct?


Solution

  • is the lock being returned guaranteed to keep the mutex locked through the process of being returned , as opposed to say unlocking then re-locking (or even unlocking and staying unlocked). I am worried about the action of the destructor of the return value.

    Yes, it will keep the lock and there is no destructor involved. The std::scoped_lock<std::mutex> is created directly in the receiving variable due to return value optimization (RVO). It's guaranteed since C++17, and you are using at least C++17.

    would std::lock_guard instead of std::scoped_lock also be correct?

    Yes, since C++17. In C++14 and earlier, it wouldn't even compile.


    An alternative could be to make your object lockable:

    struct BigObject {
        void lock() { m_mutex.lock(); }
        void unlock() { m_mutex.unlock(); }
        bool try_lock() { return m_mutex.try_lock(); }
    
    private:
        std::mutex m_mutex;
    };
    

    It would now be locked just any other mutex and the user could use whatever lock that is needed, be it a unique_lock or scoped_lock, which would be hard with your current approach:

    std::scoped_lock lock(big, another_lockable);