Assignment operator in struct after adding mutex in C++

Question

I had a struct type:

struct MyStruct {
    int field1;
    int field2;
}

Then it became necessary to add a mutex to it to make it shared between threads:

struct MyStruct {
    std::mutex _mutex;

    int field1;
    int field2;
}

But then the compiler (clang) give me these messages on lines where I assign one existing struct to variable of type MyStruct like MyStruct mystruct = p_MyStructMap->at(clientId);:

(1) error: object of type 'MyStruct' cannot be assigned because its copy assignment operator is implicitly deleted

(2) note: copy assignment operator of 'MyStruct' is implicitly deleted because field '_mutex' has a deleted copy assignment operator

std::mutex _mutex
           ^     

(3) /usr/bin/../lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/mutex:129:12: note: function has been explicitly marked deleted here

mutex& operator=(const mutex&) = delete;
       ^

Please, help: How can I rewrite struct or may be the program logic to use mutexes for this struct?

Answer 1

Assuming that _mutex was added to protect the other fields from being simultaneously accessed, you will need to lock the mutex(s) during assignment, unless you can otherwise guarantee that multiple threads aren't accessing either side of the assignment expression:

x = y;

If any other thread is reading or writing to x simultaneously, you have a race without locking.

If any other thread is writing to y simultaneously, you have a race without locking.

If you do indeed need to lock, it is not quite as simple as locking both sides:

MyStruct& operator=(const MyStruct& o)
{
    if (this != &o)
    {
        // WRONG!  DO NOT DO THIS!!!
        std::lock_guard<std::mutex> lhs_lk(_mutex);
        std::lock_guard<std::mutex> rhs_lk(o._mutex);
        field1 = o.field1;
        field2 = o.field2;
    }
    return *this;
}

The reason you should not do this is imagine thread A does this:

x = y;

And simultaneously thread B does this:

y = x;

Now you have the potential for deadlock. Imagine thread A locks x._mutex, and then thread B locks y._mutex. Now both thread A and B will block trying to lock their o._mutex, and neither thread will ever succeed because it is waiting for the other thread to release it.

The correct formulation of the assignment operator can look like this:

MyStruct& operator=(const MyStruct& o)
{
    if (this != &o)
    {
        std::lock(_mutex, o._mutex);
        std::lock_guard<std::mutex> lhs_lk(_mutex, std::adopt_lock);
        std::lock_guard<std::mutex> rhs_lk(o._mutex, std::adopt_lock);
        field1 = o.field1;
        field2 = o.field2;
    }
    return *this;
}

The job of std::lock(m1, m2, ...) is to lock all of the mutexes in some magical way that does not deadlock. For more detail than you probably want to know about how that is done, you can read Dining Philosophers Rebooted.

Now with _mutex and o._mutex locked, you simply need to make their unlocking exception safe by having the lock_guards adopt ownership of their mutexes. That is, they will no longer try to lock their mutex on construction, but they will still unlock them on destruction. The lock_guard constructors themselves throw nothing, so this is all exception safe.

Oh, you will also have to store _mutex as a mutable data member else you won't be able to lock and unlock it on the rhs.

In C++14 a potential optimization is available if you want to try it: You can "write-lock" this->_mutex and "read-lock" o._mutex. This would allow multiple threads to simultaneously assign from a common rhs if no threads are assigning to that rhs. In order to do that you need to have MyStruct store a std::shared_timed_mutex instead of a std::mutex:

#include <mutex>
#include <shared_mutex>

struct MyStruct
{
    using MutexType = std::shared_timed_mutex;
    using ReadLock = std::shared_lock<MutexType>;
    using WriteLock = std::unique_lock<MutexType>;

    mutable MutexType _mutex;

    int field1;
    int field2;

    MyStruct& operator=(const MyStruct& o)
    {
        if (this != &o)
        {
            WriteLock lhs_lk(_mutex, std::defer_lock);
            ReadLock  rhs_lk(o._mutex, std::defer_lock);
            std::lock(lhs_lk, rhs_lk);
            field1 = o.field1;
            field2 = o.field2;
        }
        return *this;
    }

};

This is similar to as before except that we need to change the type of the mutex, and now the lhs locks with unique_lock (which write-locks the lhs mutex) and the rhs locks with shared_lock (which read-locks the rhs mutex). Here we also use std::defer_lock to construct the locks but to tell the locks that the mutex isn't locked yet, and don't lock on construction. And then our old friend std::lock(m1, m2) is used to tell both locks to lock at the same time without deadlock. Yes, std::lock works with both mutex and lock types. Anything that has member lock(), try_lock(), and unlock(), will work with std::lock(m1, m2, ...).

Note, the C++14 technique isn't definitely an optimization. You will have to measure to confirm or deny that it is. For something as simple as MyStruct it probably isn't an optimization, except perhaps for some special usage patterns. The C++11 technique with std::mutex continues to be a valuable tool in the toolbox, even in C++14.

For ease in switching back and fourth between the mutex and shared_timed_mutex, this latest example makes use of type aliases, which can be easily changed. One only has to change two lines to switch back to mutex:

using MutexType = std::shared_timed_mutex;

using ReadLock = std::sharedunique_lock<MutexType>;

using WriteLock = std::unique_lock<MutexType>;