Technical Writings
- C++ Concurrency Book
- Articles and Publications
- Conference Presentations
- Recent Blog Entries
- Multithreading Blog Entries
- C++ Blog Entries
Subscribe to Blog
Blog Archives
Thread-Safe Copy and Move Constructors
Wednesday, 17 August 2011
This is a guest post by Michael Spertus. Michael is a Distinguished Engineer at Symantec. He is also a C++ Standards Committee member and teaches the graduate C++ sequence at the University of Chicago. He can be contacted at mike_spertus@symantec.com.
This guest column discusses writing thread-safe constructors. As we will see, this is more difficult than it seems. Fortunately, we will also see that C++11 offers a very pretty solution to this problem that nicely illustrates the synergy of the new features introduced in C++11.
The problem
If you have a class that supports locking of objects to serialize access to a given object, you probably want the class' copy constructor and move constructor (if it has one) to lock the source object to get a consistent snapshot of the source object so the destination object isn't messed up if the source changes in the middle of the copy or move.
This isn't nearly as easy as it sounds. In the following class, a
mutex is used to try to enforce the invariant that i_squared
should always be the square of i.
class A {
public: A(_i = 0) { set(_i); }
set(int _i) {
std::lock_guard<std::mutex> lock(mtx);
i = _i;
i_squared = i*i;
}
...
private:
std::mutex mtx;
int i;
int i_squared;
};
Unfortunately, the default copy constructor doesn't acquire the
mutex, so in code like the following, f
can copy a "half set" version of a
if another thread modifies a
at the same time.
void f(A &a)
{
A a2 = a;
...
}
First attempt
A naive attempt is to acquire the lock in the constructor body just like in a thread-safe method.
class A {
public:
A(const A &a) : i(a.i), i_squared(a.i_squared) {
std::lock_guard<std::mutex> lock(a.mtx); // Too late!
}
...
};
Unfortunately, this fares no better
as i and i_squared are
copied before we acquire the lock.
Second attempt
One approach would be to simply not lock in the copy constructor at all and just manually lock objects you want to copy:
void f(A &a)
{
std::lock_guard<std::mutex>
lock(a.mtx);
A a2 = a;
...
}
This approach deserves careful consideration. For classes which
are not usually shared between threads or which need locking granularity
at a different level than their internal operations, managing locks
within the class can be an antipattern. This concern was a primary reason
why C++11 does not have an equivalent to the SynchronizedCollection
wrapper found in Java and C#. For example, synchronized
collections make it easy to inadvertently loop through a collection believing
your code is thread-safe even though the collection could change between
individual operations on the collection during the loop. Of course, if
we decide not to have A's copy constructor lock, then A::set()
should not lock either.
Still, it remains a very common and useful pattern for classes designed for shared
use to have all their internal operations acquire the
lock (i.e.,
monitors/synchronized classes).
If A is a synchronized class that locks its methods internally, it would
be very confusing and prone to intermittent errors to still have to manually acquire
the
lock whenever an object is copied or moved. Also, generic
code, which doesn't know about A::mtx is unlikely to work
properly.
Third attempt
One thing we can do is dispense with member initialization lists in constructors altogether
class A {
public:
A(const A &a) {
std::lock_guard<std::mutex> lock(a.mtx);
i = a.i;
i_squared = a.i_squared;
}
...
};
This solution is awkward at best if any bases or members don't have default constructors, have reference type, or are const. It also seems unfair to have to pay an efficiency penalty (for constructing and assigning separately) just because there is no place to put the lock. In practice, I also suspect intermittent errors will creep into large code bases as programmers carelessly add a base or member initializer to the constructor. Finally, it just isn't very satisfying to have to just discard core parts of constructor syntax just because your class is synchronized.
Fourth attempt
Anthony Williams has suggested implementing serialized classes using a wrapper class like:
struct PrivateBaseForA
{
int i;
int i_squared;
};
class A: private PrivateBaseForA
{
mutable std::mutex mtx;
public:
A(int _i = 0) { set(_i); }
void set(int _i) {
std::lock_guard<std::mutex> lock(mtx);
i = _i;
i_squared = _i*_i;
}
A(const A& other):
PrivateBaseForA((std::lock_guard<std::mutex>(other.mtx),other))
{}
};
Anthony makes slick use of double-parens to use the comma operator. If you
wanted to avoid this, you could have PrivateBaseForA's constructor take a lock.
Again, this is not yet a very satisfying solution because writing a wrapper class for every synchronized class just to get a properly locked copy constructor is clumsy and intrusive.
Finally, the C++11 approach
Fortunately, C++11 offers a nice solution that really illustrates how beautifully and powerfully the features of C++11 work together. C++11 supports forwarding one constructor to another, which gives us an opportunity to grab the mutex before any copying or moving takes place:
class A {
private:
A(const A &a, const std::lock_guard<std::mutex> &)
: i(a.i), i_squared(a.i_squared) {}
public:
A(const A &a) : A(a, std::lock_guard<std::mutex>(a.mtx)) {}
...
};
This solution locks the entire constructor, protects against races resulting from forgetting the manual lock, works with generic code, and doesn't require the creation of artificial wrapper classes.
While I think this is clearly the preferred solution, it is not perfect. To begin with, it is more obscure and guru-like than I would wish for such a common situation. Secondly, constructor forwarding is not yet implemented by any major C++ compiler (although the recently approved standard should change that soon). Finally, we lose the benefit of compiler generated copy constructors. If we added an i_cubed field to A that also needed to be kept consistent with i, we might forget to update the private constructor. Perhaps this will be a further opportunity for the next C++ standard (C++1y?). In the meantime, C++11 provides a powerful new solution to the everyday problem of writing thread-safe copy and move constructors.
One final note is to mention that although this article focused on copy constructors, everything applies equally to move constructors. Indeed, any constructor that needs to acquire any lock whatsoever (e.g., a database lock) for its entire duration can apply these techniques.
Posted by Anthony Williams
[/ threading /] permanent link
Tags: multithreading, copying
Stumble It! 
| Submit to Reddit 
| Submit to DZone 
If you liked this post, why not subscribe to the RSS feed 
or Follow me on Twitter? You can also subscribe to this blog by email using the form on the left.
Design and Content Copyright © 2005-2024 Just Software Solutions Ltd. All rights reserved. | Privacy Policy
19 Comments
There is another way to write thread-safe copy-constructors in C++03.
class Locker { public: Locker(std::mutex& _mtx) : mtx(&_mtx) { _mtx.lock(); } ~Locker() { if (mtx) mtx->unlock(); } // Must be called at the end of the // copy constructor. Signals that copy // construction succeeded. void Finalize() { mtx->unlock(); mtx = 0; } private: std::mutex* mtx; };
https://dzone.com/articles/3-top-android-app-development-trends-to-watch-out
There is something that makes me uneasy with this solution. Even though it's correct and reasonably safe, it breaks one of the principles of ownership. Object A is a monitor--it protects data it owns from concurrent access using a built-in mutex. The principle is that only the owner should lock its data. Here, the nascent object locks data that it doesn't own. In general, accessing somebody else's mutex not only breaks encapsulation, but puts one on a slippery slope towards deadlocks. Of course an argument can be made that in this case a deadlock is impossible, but you can't make this argument from first principles--you have to go outside of the ownership model.
How would I solve this problem? By prohibiting copy construction of A and instead providing the clone method. The cloner, in this case, locks its own data for the duration of cloning.
@Bartosz: IMO the copy constructor of a class already violates the principle of ownership of private data to an extent. If it did not, direct access to members in the object being copied would be impossible (or at least error-prone and full of nasty-looking code). It is mitigated somewhat by the fact that the copied object is const (ensuring that at least the objects invading its privacy will not leave a path of destruction in their wake), but in a move constructor, the copying object is given free reign to mutate the copied object however it pleases. Locking its mutex is nothing compared to wiping out all of its encapsulated data. The copyable concept implies that an object consents to having other instances of the same class peer into its innermost data (and since the mutex is in this pattern usually also a mutable member, access to it is also implied), and the movable concept implies that an object consents to losing control over anything it was protecting. These can be both desirable and undesirable, which is probably why they are not mandatory and indeed easy to forbid in the class declaration.
There is another way to write thread-safe copy-constructors in C++03.
class Locker { public: Locker(std::mutex& _mtx) : mtx(&_mtx) { _mtx.lock(); } ~Locker() { if (mtx) mtx->unlock(); } // Must be called at the end of the // copy constructor. Signals that copy // construction succeeded. void Finalize() { mtx->unlock(); mtx = 0; } private: std::mutex* mtx; };
class A : private Locker { public: A(const A &a) : Locker(a.mtx), i(a.i), i_squared(a.i_squared) { Finalize(); } ... private: std::mutex mtx; int i; int i_squared; };
It increases the size of A by one pointer, though. If A's copy constructor can't throw, the overhead can be removed.
The advantage of this approach compared to the one suggested by Anthony Williams is that it doesn't require an extra class each time you want to write a thread-safe copy constructor.
The formatting in my post got screwed up. Here's the same code on pastebin: http://pastebin.com/5cjMpHmH.
@Bartosz: I tend to agree with Michel that if you're going to allow copying at all, then having the copy constructor lock the mutex is not a big deal. Providing a clone function is a valid alternative, but doesn't work nicely in all cases: if the clone function returns by value then you still need a public copy or move constructor to allow the use to capture the new instance.
@Roman: Your suggestion of using a "Locker" base class is a good one. I don't like the compulsory "Finalize()" call or the space overhead, but it is a viable option, and certainly less typing than my suggestion.
Thanks for all the interesting comments. A few thoughts:
@Roman I had rejected the locker base class approach both because requiring all constructors to manually unlock seems error-prone and because order of base class initialization can be tricky. For example, if A also inherits from B and B has a virtual base class, than that will be constructed before Locker. You can fix this, but it illustrates the potential for subtle errors (esp. in generic code).
In any case, it is a very interesting idea, and the moment I posted the article, I regretted not including it in the list of approaches, so I definitely want to thank you for bringing it up.
@Bartosz Interesting thoughts. While I'm locking from a different object, all locking is confined to class members, which is still an easily understood coding practice. As a result, I'm not sure my approach is any more difficult to analyze than clone. Even the standard practice in both C# and Java of manually locking SerializedCollections when a sequence of operations need to be performed atomically (e.g., when iterating over the collection) has turned out to be usable in practice...
My primary concern about using clone() is simply that copy constructors are the idiomatic approach to copying in C++ just as clone() is in Java. E.g., there is an is_copy_constructible type trait that generic code will use to see if an object is copyable but there is no is_clonable type trait. FWIW, You'd need another non-idiomatic method to cover move construction.
Thanks again,
Mike
Hi Antti, That's an interesting idea, but isn't it non-conforming? In both the C++03 and C++11 standards, 8.3.6p9 says:
... parameters of a function shall not be used in a default argument, ... int f(int a, int b = a); // error: parameter a used as default argument
Does your compiler accept your code? If so, why doesn't it conflict with the above excerpt? (BTW, the stated reason for this restriction is to avoid interfering with the undefined-ness of the order of argument evaluation).
Best,
MikeYou're absolutely right, damnit. :)
I thought I was being clever but C++ turned out to be more clever than me. I didn't actually run it through a compiler so I didn't catch it. That's what I get for wrestling with C++ after midnight.
You can also do the locking and unlocking manually, rather than using lock_guard, for something which works in C++03:
A(const A &other) : i((other.mtx.lock(), other.i)), i_squared(other.i_squared) { other.mtx.unlock(); }
This won't work if you need to call a base constructor with more than one argument referring to other, though.
You could call a method to initialize the first member and lock the source there. It works with current compilers, but would not synchronize base members. You would also manually need to unlock and it's not exception safe. Could be made exception safe with an overhead of a pointer member of type lock_guard.
It's best to make the locked_init generic, to be usable with any member type.
template<class T> T locked_init(std::mutex & mtx, const T & t) { mtx.lock(); return t; }
class A { public: A(int i = 0) : i(0), i_squared(i * i) {} A(const A & other) : i(locked_init(other.mtx, other.i)), i_squared(other.i_squared) { other.mtx.unlock(); }
private:
mutable std::mutex mtx; int i; int i_squared; };
Technically, you are right, there are correct ways of providing a copy constructor to a monitor object (an object with its own mutex). My objections are non-technical. A copy-constructible object is a generalization of a plain value. A value object is the antithesis of sharing: every time you pass it, you get your own copy.
Monitors, on the other hand, are specifically designed for sharing. Making monitors copy-constructible is confusing and error prone. Probably this is why std::mutex is specifically non-copy-constructible, so you can't default-copy monitors.
Move constructor, on the other hand, is a completely different story. If you have to lock the source object during move, you already have a bug in your program. Before you move an object you have to guarantee that nobody else has access to it, especially another thread. Move semantics implements the idea of unique objects, and one of the properties of unique objects is that the source is invalid after the move (think std::unique_ptr).
I came up with a different solution using C++11.
https://github.com/toffaletti/libten/blob/master/include/ten/synchronized.hh
example usage:
https://github.com/toffaletti/libten/blob/master/tests/test_qutex.cc#L47
I haven't actually used this code for anything, but it seems like it might be useful.
@Bartosz - You are absolutely right about the move constructor! I don't know why I made the comment about the idiom also being useful for move constructors. Thanks for pointing that out. Here is a typical example of where a copyable monitor is useful: Suppose you have a logically immutable type designed for concurrent use that is not physically immutable (i.e., a tree that uses mutable fields to rebalance itself on reads). It needs to be a monitor (i.e., lock its methods) because it needs to remain physically consistent during method calls. As you point out, when monitors are just being used as synchronization objects, like mutex, they are generally not copyable.
@Attila - You are correct that Peter's code is not exception-safe (good catch!), although I don't see that yours is either. Roman's code provides an exception-safe solution along these lines.
Is it safe to assume, that the temporary that is created by the is really destroyed before the next copy occurs in _one_ expression? A temporary is destroyed when the `;` is reached. What if two copies are tried on one line.
Maybe something like:
"""A a; A b{a}, c{a};"""
or
""" void func(A x, A y);
int main() { A a; func(a, a); } """