How to implement an atomic (thread-safe) and exception-safe deep copy assignment operator?

Question

I was asked this question in an interview, and I couldn't answer it well.

More specifically, the class to which the assignment operator belongs looks like this:

class A {
private:
    B* pb;
    C* pc;
    ....
public:
    ....
}

How to implement an atomic (thread-safe) and exception-safe, deep copy assignment operator for this class?

Answer 1

There are two separate concerns (thread-safety and exception-safety) and it seems best to address them separately. To allow constructors taking another object as argument to acquire a lock while initializing the members, it is necessary to factor the data members into a separate class anyway: This way a lock can be acquired while the subobject is initialized and the class maintaining the actual data can ignore any concurrency issues. Thus, the class will be split into two parts: class A to deal with concurrency issues and class A_unlocked to maintain the data. Since the member functions of A_unlocked don't have any concurrency protection, they shouldn't be directly exposed in the interface and, thus, A_unlocked is made a private member of A.

Creating an exception-safe assignment operator is straight forward, leveraging the copy constructor. The argument is copied and the members are swapped:

A_unlocked& A_unlocked::operator= (A_unlocked const& other) {
    A_unlocked(other).swap(*this);
    return *this;
}

Of course, this means that a suitable copy constructor and a swap() member are implemented. Dealing with the allocation of multiple resources, e.g., multiple objects allocated on the heap, is easiest done by having a suitable resource handler for each of the objects. Without the use of resource handlers it becomes quickly very messy to correctly clean up all resources in case an exception is thrown. For the purpose of maintaining heap allocated memory std::unique_ptr<T> (or std::auto_ptr<T> if you can't use C++ 2011) is a suitable choice. The code below just copies the pointed to objects although there isn't much point in allocating the objects on the heap rather than making them members. In a real example the objects would probably implement a clone() method or some other mechanism to create an object of the correct type:

class A_unlocked {
private:
    std::unique_ptr<B> pb;
    std::unique_ptr<C> pc;
    // ...
public:
    A_unlocked(/*...*/);
    A_unlocked(A_unlocked const& other);
    A_unlocked& operator= (A_unlocked const& other);
    void swap(A_unlocked& other);
    // ...
};

A_unlocked::A_unlocked(A_unlocked const& other)
    : pb(new B(*other.pb))
    , pc(new C(*other.pc))
{
}
void A_unlocked::swap(A_unlocked& other) {
    using std::swap;
    swap(this->pb, other.pb);
    swap(this->pc, other.pc);
}

For the thread-safety bit it is necessary to know that no other thread is messing with the copied object. The way to do this is using a mutex. That is, class A looks something like this:

class A {
private:
    mutable std::mutex d_mutex;
    A_unlocked         d_data;
public:
    A(/*...*/);
    A(A const& other);
    A& operator= (A const& other);
    // ...
};

Note, that all members of A will need to do some concurrency protection if the objects of type A are meant to be used without external locking. Since the mutex used to guard against concurrent access isn't really part of the object's state but needs to be changed even when reading the object's state, it is made mutable. With this in place, creating a copy constructor is straight forward:

A::A(A const& other)
    : d_data((std::unique_lock<std::mutex>(other.d_mutex), other.d_data)) {
}

This locks the argument's mutex and delegates to the member's copy constructor. The lock is automatically released at the end of the expression, independent of whether the copy was successful or threw an exception. The object being constructed doesn't need any locking because there is no way that another thread knows about this object, yet.

The core logic of the assignment operator also just delegates to the base, using its assignment operator. The tricky bit is that there are two mutexes which need to be locked: the one for the object being assigned to and the one for the argument. Since another thread could assign the two objects in just the opposite way, there is a potential for dead-lock. Conveniently, the standard C++ library provides the std::lock() algorithm which acquires locks in an appropriate way that avoids dead-locks. One way to use this algorithm is to pass in unlocked std::unique_lock<std::mutex> objects, one for each mutex needed to be acquired:

A& A::operator= (A const& other) {
    if (this != &other) {
        std::unique_lock<std::mutex> guard_this(this->d_mutex, std::defer_lock);
        std::unique_lock<std::mutex> guard_other(other.d_mutex, std::defer_lock);
        std::lock(guard_this, guard_other);

        *this->d_data = other.d_data;
    }
    return *this;
}

If at any point during the assignment an exception is thrown, the lock guards will release the mutexes and the resource handlers will release any newly allocated resource. Thus, the above approach implements the strong exception guarantee. Interestingly, the copy assignment needs to do a self-assignment check to prevent locking the same mutex twice. Normally, I maintain that a necessary self-assignment check is an indication that the assignment operator isn't exception safe but I think the code above is exception safe.

This is a major rewrite of the answer. Earlier versions of this answer were either prone to a lost update or to a dead-lock. Thanks to Yakk for pointing out the problems. Although the result of addressing the issues involves more code, I think each individual part of the code is actually simpler and can be investigated for correctness.

Answer 2

First, you must understand that no operation is thread safe, but rather all operations on a given resource can be mutually thread safe. So we must discuss the behavior of non-assignment operator code.

The simplest solution would be to make the data immutable, write an Aref class that uses the pImpl class to store an immutable reference counted A, and have mutating methods on Aref cause a new A to be created. You can achieve granularity by having immutable reference counted components of A (like B and C) follow a similar pattern. Basically, Aref becomes a COW (copy on write) pImpl wrapper for an A (you can include optimizations to handle single-reference cases to do away with the redundant copy).

A second route would be to create a monolithic lock (mutex or reader-writer) on A and all of its data. In that case, you either need mutex ordering on the locks for instances of A (or a similar technique) to create a race-free operator=, or accept the possibly surprising race condition possibility and do the copy-swap idiom Dietmar mentioned. (Copy-move is also acceptable) (Explicit race condition in the lock-copyconstruct, lock-swap assignment operator=: Thread1 does X=Y. Thread 2 does Y.flag = true, X.flag = true. State afterwards: X.flag is false. Even if Thread2 locks both X and Y over the entire assignment, this can happen. This would surprise many programmers.)

In the first case, non-assignment code has to obey the copy-on-write semantics. In the second case, non-assignment code has to obey the monolithic lock.

As for exception safety, if you presume your copy constructor is exception safe, as is your lock code, the lock-copy-lock-swap one (the second) is exception safe. For the first one, so long as your reference counting, lock clone and data modification code is exception safe you are good: the operator= code is pretty brain dead in either case. (Make sure your locks are RAII, store all allocated memory in a std RAII pointer holder (with the ability to release if you end up handing it off), etc.)