c++visual-c++lockingreturn-by-valuelock-guard

Why can't I return a std::lock_guard? How can I work around it?


I'm trying to return a std::lock_guard by value. The following works fine using an online compiler (programiz), but in Visual Studio 2017 using C++17, lock1() below won't compile because I'm attempting to reference a deleted function and I have to work around it using std::shared_ptr:

#include <mutex>
#include <memory>

std::lock_guard<std::mutex> lock1(std::mutex& m)
{
    return std::lock_guard<std::mutex>{ m };
}

std::shared_ptr<std::lock_guard<std::mutex>> lock2(std::mutex& m)
{
    return std::make_shared<std::lock_guard<std::mutex>>(m);
}

int main()
{
    std::mutex m1;
    auto lg1 = lock1(m1);
    std::mutex m2;
    auto lg2 = lock2(m2);

    return 0;
}

Am I doing something wrong, or is MSVC at fault? Can I work around it without using a std::shared_ptr, for example using std::move?

Context: I'm trying to make a thread-safe wrapper template for more convenient mutex locking. I decided to make it a "pointer" but that's inessential. Here's what I've come up with. Note the commented-out variant of lock().

#include <mutex>
#include <memory>
#include <map>
#include <utility>
#include <thread>

template<typename T>
class ThreadSafePtr
{
    std::shared_ptr<T> ptr;
    std::shared_ptr<std::mutex> mutex;

public:
    template<typename... Args>
    ThreadSafePtr(Args&&... args) : ptr{ std::make_shared<T>(std::forward<Args>(args)...) }, mutex{ std::make_shared<std::mutex>() } {}
    ThreadSafePtr(ThreadSafePtr& orig) = default;
    ThreadSafePtr& operator= (ThreadSafePtr& orig) = default;

    class Lock
    {
        friend class ThreadSafePtr<T>;
        T& ref;
        std::lock_guard<std::mutex> lock;
        Lock(ThreadSafePtr* tsptr) : ref{ *tsptr->ptr }, lock{ *tsptr->mutex } {}
    public:
        T& get() const { return ref; }
    };

    //Lock lock() { return Lock{ this }; }
    std::shared_ptr<Lock> lock()
    {
        return std::shared_ptr<Lock>{ new Lock{ this } };
    }
};

int main()
{
    ThreadSafePtr<std::map<int, int>> map;
    std::thread([map]() mutable -> void { map.lock()->get()[1] = 2; }).detach();
    std::thread([map]() mutable -> void { map.lock()->get()[3] = 4; }).detach();

    return 0;
}

(Also, depending on the compiler, forwarding the arguments in the constructor may cause compiler error (but only if a ThreadSafePtr is being captured in a lambda), not sure what's up with that, but removing std::forward<Args>(args)... gets rid of that problem. Removing the copy constructor and copy assignment operator also gets rid of it... and replacing the map with std::map<std::string, std::string> may cause compiler error... lots of exciting possibilities...)


Solution

  • Your code compiles on my VS2022, so I assume they fixed some C++17 compliancy issues as @NathanOliver commented.

    But anyway the issue is that std::lock_guard is neither copyable nor movable. Therefore you can return is by value only if RVO is taking place (as it should in this case).

    Because std::lock_guard is not even movable, using std::move will not help if RVO is not taking place.

    However - as @MilesBudnek commented, you can use std::unique_lock instead. Unlike std::lock_guard it is movable, and so you can use std::move to return it by moving into the caller's object.