HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Copyable Atomic

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
copyableatomicstackoverflow

Problem

I find it pretty annoying that c++11 atomics can't be copied. The reasons for this have been discussed e.g. here and I don't want to argue about them now. However, I find myself repeatedly in situations, where I want to copy data structures around that contain atomics. Usually BEFORE they are actually used in a multithreaded context (e.g. in order to return them from a factory function, or to store them in a vector etc.).

In order to solve that problem without having to manually write a copy constructor over and over again, I decided to write a simple class, that publicly derives from std::atomic and adds those functionality:

/**
 * Drop in replacement for std::atomic that provides a copy constructor and copy assignment operator.
 *
 * Contrary to normal atomics, these atomics don't prevent the generation of 
 * default constructor and copy operators for classes they are members of.
 * 
 * Copying those atomics is thread safe, but be aware that 
 * it doesn't provide any form of synchronization.
 */
template
class CopyableAtomic : public std::atomic
{
public:
    //defaultinitializes value
    CopyableAtomic() : 
        std::atomic(T{})
    {}

    constexpr CopyableAtomic(T desired) : 
        std::atomic(desired) 
    {}

    constexpr CopyableAtomic(const CopyableAtomic& other) :
        CopyableAtomic(other.load(std::memory_order_relaxed))
    {}

    CopyableAtomic& operator=(const CopyableAtomic& other) {
        this->store(other.load(std::memory_order_relaxed), std::memory_order_relaxed);
        return *this;
    }
};


In my toy examples this worked pretty well, however, I'm not sure, if I really considered all possible ramifications of this - deriving from standard library types feels awkward enough and when it comes to synchronization primitives I feel a bit like playing with fire (or juggling with razorblades to quote Herb Sutter).

So what I would like to know (aside from general improvement suggestions, or alternative approaches):

-

Solution

Difference in default constructor

std::atomic's default constructor is trivial - yours isn't. If your goal is to simply add copy semantics, I would maintain this same behavior:

CopyableAtomic() = default;


Memory orders

I'm not sure about relaxed here. You probably want to ensure consistent orderings where you use this object, so I would change the load()s to use std::memory_order_acquire and the store() to use std::memory_order_release. That is:

CopyableAtomic& operator=(const CopyableAtomic& other) {
    this->store(
        other.load(std::memory_order_acquire),
        std::memory_order_release);
    return *this;
}


Otherwise, you might get unexpected reorderings. Better to be on the safe side if you're writing a class like this.

Otherwise

This looks perfectly fine to me. With the exception of copying, this is precisely a std::atomic so it should have all the same behavior everywhere - so it should be able to be a straight-forward drop in replacement in all places.

Code Snippets

CopyableAtomic() = default;
CopyableAtomic& operator=(const CopyableAtomic<T>& other) {
    this->store(
        other.load(std::memory_order_acquire),
        std::memory_order_release);
    return *this;
}

Context

StackExchange Code Review Q#113439, answer score: 7

Revisions (0)

No revisions yet.