Thread safe holder

I have implemented a thread safe holder to safely pass data between threads.

User can set value many times, but only the first SetIfEmpty call stores the value, then user may read the value many times.

template 
class ThreadSafeHolder {
public:
    ThreadSafeHolder() : is_value_set_(false) {
    }

    void SetIfEmpty(const T& value) {
        std::lock_guard lock(mutex_);
        // memory_order_relaxed is enough because storing to
        // `is_value_set_` happens only in `SetIfEmpty` methods
        // which are protected by mutex.
        if (!is_value_set_.load(std::memory_order_relaxed)) {
            new(GetPtr()) T(value);
            is_value_set_.store(true, std::memory_order_release);
        }
    }

    void SetIfEmpty(T&& value) {
        std::lock_guard lock(mutex_);
        if (!is_value_set_.load(std::memory_order_relaxed)) {
            new(GetPtr()) T(std::move(value));
            is_value_set_.store(true, std::memory_order_release);
        }
    }

    //! This method might be safely call only if previous `IsEmpty()`
    //! call returned `false`.
    const T& Get() const {
        assert(!IsEmpty());
        return *GetPtr();
    }

    bool IsEmpty() const {
        // memory_order_acquire loading to become synchronize with
        // memory_order_release storing in `SetIfEmpty` methods.
        return !is_value_set_.load(std::memory_order_acquire);
    }

    ~ThreadSafeHolder() {
        if (!IsEmpty()) {
            GetPtr()->~T();
        }
    }

private:
    T* GetPtr() {
        return reinterpret_cast(value_place_holder_);
    }

    const T* GetPtr() const {
        return reinterpret_cast(value_place_holder_);
    }

    // Reserved place for user data.
    char value_place_holder_[sizeof(T)];
    // Mutex for protecting writing access to placeholder.
    std::mutex mutex_;
    // Boolean indicator whether value was set or not.
    std::atomic is_value_set_;
};

Questions

• Is the code correct in general?

• Is access

Code looks good. I only have a few comments.

Alignment

Our data is stored thusly:

char value_place_holder_[sizeof(T)];

This has alignment 1. You really want it to have the same alignment as T. That's easy enough:

using Storage = std::aligned_storage_t;
Storage data;

Usability

Right now, you have two setting functions:

void SetIfEmpty(const T& value) { ... }
void SetIfEmpty(T&& value) { ... }

But if you're already constructing using placement new, we could just allow for arbitrary arguments. It'll let your users do more with what you have:

template 
void SetIfEmpty(Args&&... args) {
    ...
    new (GetPtr()) T(std::forward(args)...);
    ...
}

That'll also reduce the code duplication on your copy and move construction.

Unnecessary Mutex

You have a std::atomic, let's take advantage of it. There's a function called compare_exchange_strong with which we can do:

template 
void SetIfEmpty(Args&&... args) {
    bool expected = false;
    if (is_value_set_.compare_exchange_strong(expected, true))
    {
        new (GetPtr()) T(std::forward(args)...);
    }
}

That is a single atomic operation. We load the value of is_value_set_ and if it was false, we set it to true and the function returns true. If two threads get to that line at the same time, only one of them will succeed in flipping it to true and the other will fail.

This also gives us...

Copyable? Movable?

Dropping the mutex means our class is tentatively copyable and movable. Do we want to support that? If so, the default behavior is bitwise copy and move which is fine for PODs but not for anything else. You would have to add those operations.

StackExchange Code Review Q#105742, answer score: 3