C++ settings return value mutex

In my application running on an ESP32 using FreeRTOS, I've implemented a setting class responsible for storing various settings. I'm currently concerned about the thread safety of this class, specifically regarding the 'Get' method.

Could someone please review my implementation and confirm whether it is correct and thread-safe? I have doubts about whether 'copy' is always a true copy, especially when the setting type is a string or an object.

template<typename T>
class Setting : public ISetting
{
    Mutex& mutex;
    T Value;

public:
    Setting(Mutex& mutex, const T& defaultValue)
        : mutex(mutex)
        , Value(defaultValue)
    {
    }

    void Set(const T& value)
    {
        mutex.Take();
        Value = value; // Assign the new value to the setting
        mutex.Give();
    }

    T Get()
    {
        T copy;
        mutex.Take();
        copy = Value;
        mutex.Give();
        return copy;
    }

    void Accept(ISettingVisitor* handler)
    {
        handler->Visit(this);
    }
};

Get() is thread-safe in the sense that the execution of the copy assignment operator for T is only done by one thread concurrently. However, it's not safe at large.

Exception Safety

Take() and Give() are really unusual names for locking/unlocking; for std::mutex these are called .lock() and .unlock(). If they were called like that, you could use std::scoped_lock instead of manually locking/unlocking:

void Set(const T& value)
{
    std::scoped_lock lock{mutex}; // or std::lock_guard before C++17
    Value = value;
}

T Get()
{
    std::scoped_lock lock{mutex}; // or std::lock_guard before C++17
    return Value;
}

Or alternatively, this could use std::experimental::scope_exit or some other way to unlock the mutex in the destructor.

In any case, it's really important that you unlock the mutex in a destructor because that makes exception safety easy. mutex in your code is never unlocked if copy = Value throws an exception, and a copy assignment operator could reasonably do that. The same issues apply to Set().

Unsafe if T has reference semantics

If T doesn't have value semantics, then this code still isn't really thread-safe. For example, if T = std::span, then the caller could modify the data that the span references concurrently and unsafely. Copying a std::span is what you would call a "shallow copy"; it doesn't copy the underlying data ("deep copy").

However, that's not something that you can fix. There is no way to check what the semantics of a given type are. It's the responsibility of the user of Setting to ensure overall safe usage. The responsibility of Setting is only to thread-safely get and set some value.