patterncppMinor
Copyable Atomic
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
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):
-
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
Memory orders
I'm not sure about
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'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.