Say I define a toy class that I want to claim is thread-safe.
(Added following long discussion in comments: This a thought experiment about a race that I believe is commonly present and largely benign, but nevertheless real. It is between the initialization of a data member in a constructor, and its otherwise correct use in another thread that did not synchronize with the constructor. I am looking for a direct solution to the problem that would likely need to be in the constructor. It is easy to work-around and avoid this problem in practice.)
class Blah {
public:
Blah () { m_blah = 123; }
int blah () {
std::lock_guard _{m_mutex};
m_blah += 1;
return m_blah;
}
private:
std::mutex m_mutex;
int m_blah;
};
Now there is nothing to stop someone else doing the following ...
std::atomic<Blah *> ptr{nullptr};
ptr.store (new Blah(), std::memory_order_relaxed)
..., and accessing a freshly-made Blah from another thread that is already running (i.e. there is no synchronization as the second thread starts).
if (auto p = ptr.load()) p->blah();
They see a race between the initialization of m_blah and the critical section protected by the mutex. They blame me because I said the class was thread-safe, and it broke when used in at least a technically legal way.
Should I
not worry about it because the user had it coming (and everyone else would also call the class thread-safe)
take the mutex in the constructor to synchronize the memory (and hope that works, which it might not because the state of the mutex might not be synchronized), or
synchronize the memory inside the class in some other way?
Assuming I wanted to go with 3) even though that would presumably be unusual, what would be the best way?
Partially-constructed objects aren't "thread-safe" in C++ in general, and nobody should have any expectation that they are. Don't publish them to other threads until they're fully constructed (e.g. by using a release store instead of relaxed). Not even std::atomic<T> itself has a thread-safe constructor.
Solution 1 is the only sane way: don't try to work around buggy code using your class.
Solution 3 is I think possible by including std::atomic_thread_fence(release) or seq_cst at the bottom of the constructor, to promote any later relaxed store to release wrt. the earlier operations in the constructor, such as constructing the std::mutex.
That wouldn't help with placement-new when another thread already has a reference to the memory this thread is constructing the object in, but for regular new it could work around buggy callers.
Making code less efficient (that fence is only free on x86) to work around hypothetical and probably-obvious bugs is bad design for C++. If people didn't care about performance, they shouldn't be using C++ and threads. Compile with -fsanitize=thread to catch stuff like this.
It's a bad idea for one thread to "baby" users and try to make it safe anyway because there are other classes whose constructors aren't thread-safe. It's a bad habit for a programmer to ever write x.store(new anything, relaxed); that's always a bug unless there are no other threads that could read x yet.
(In the still-private case, relaxed is a good thing: do the ordering when later publishing a reference to x or the object it's part of. Or if another thread isn't started yet, thread creation synchronizes with the parent. In such cases you might consider std::atomic_ref<T> to allow non-atomic access to objects when there aren't other threads. This can especially help if there are multiple contiguous objects that might let a compiler use SIMD vectors to zero or copy multiple of them, which relaxed atomic<T> would still defeat.)
Solution 2 can't work - std::mutex's constructor is not AFAIK thread-safe; other threads must not start using the mutex until its constructor finishes.
So taking (and releasing? or not?) the mutex inside the constructor wouldn't help; a relaxed store can still publish a pointer that would let other threads potentially access the class even before the mutex was constructed, or before the constructor locked its own mutex if you avoided the first problem.