How should I deal with mutexes in movable types in C++?

Question

By design, std::mutex is not movable nor copyable. This means that a class A holding a mutex won't receive a default move constructor.

How would I make this type A movable in a thread-safe way?

Answer 1

Given there doesn't seem to be a nice, clean, easy way to answer this - Anton's solution I think is correct but its definitely debatable, unless a better answer comes up I would recommend putting such a class on the heap and looking after it via a std::unique_ptr:

auto a = std::make_unique<A>();

Its now a fully movable type and anyone who has a lock on the internal mutex whilst a move happens is still safe, even if its debatable whether this is a good thing to do

If you need copy semantics just use

auto a2 = std::make_shared<A>();

Answer 2

This is an upside-down answer. Instead of embedding "this objects needs to be synchronized" as a base of the type, instead inject it under any type.

You deal with a synchronized object very differently. One big issue is you have to worry about deadlocks (locking multiple objects). It also should basically never be your "default version of an object": synchronized objects are for objects that will be in contention, and your goal should be to minimize contention between threads, not sweep it under the rug.

But synchronizing objects is still useful. Instead of inheriting from a synchronizer, we can write a class that wraps an arbitrary type in synchronization. Users have to jump through a few hoops to do operations on the object now that it is synchronized, but they are not limited to some hand-coded limited set of operations on the object. They can compose multiple operations on the object into one, or have an operation on multiple objects.

Here is a synchronized wrapper around an arbitrary type T:

template<class T>
struct synchronized {
  template<class F>
  auto read(F&& f) const&->std::result_of_t<F(T const&)> {
    return access(std::forward<F>(f), *this);
  }
  template<class F>
  auto read(F&& f) &&->std::result_of_t<F(T&&)> {
    return access(std::forward<F>(f), std::move(*this));
  }
  template<class F>
  auto write(F&& f)->std::result_of_t<F(T&)> {
    return access(std::forward<F>(f), *this);
  }
  // uses `const` ness of Syncs to determine access:
  template<class F, class... Syncs>
  friend auto access( F&& f, Syncs&&... syncs )->
  std::result_of_t< F(decltype(std::forward<Syncs>(syncs).t)...) >
  {
    return access2( std::index_sequence_for<Syncs...>{}, std::forward<F>(f), std::forward<Syncs>(syncs)... );
  };
  synchronized(synchronized const& o):t(o.read([](T const&o){return o;})){}
  synchronized(synchronized && o):t(std::move(o).read([](T&&o){return std::move(o);})){}  
  // special member functions:
  synchronized( T & o ):t(o) {}
  synchronized( T const& o ):t(o) {}
  synchronized( T && o ):t(std::move(o)) {}
  synchronized( T const&& o ):t(std::move(o)) {}
  synchronized& operator=(T const& o) {
    write([&](T& t){
      t=o;
    });
    return *this;
  }
  synchronized& operator=(T && o) {
    write([&](T& t){
      t=std::move(o);
    });
    return *this;
  }
private:
  template<class X, class S>
  static auto smart_lock(S const& s) {
    return std::shared_lock< std::shared_timed_mutex >(s.m, X{});
  }
  template<class X, class S>
  static auto smart_lock(S& s) {
    return std::unique_lock< std::shared_timed_mutex >(s.m, X{});
  }
  template<class L>
  static void lock(L& lockable) {
      lockable.lock();
  }
  template<class...Ls>
  static void lock(Ls&... lockable) {
      std::lock( lockable... );
  }
  template<size_t...Is, class F, class...Syncs>
  friend auto access2( std::index_sequence<Is...>, F&&f, Syncs&&...syncs)->
  std::result_of_t< F(decltype(std::forward<Syncs>(syncs).t)...) >
  {
    auto locks = std::make_tuple( smart_lock<std::defer_lock_t>(syncs)... );
    lock( std::get<Is>(locks)... );
    return std::forward<F>(f)(std::forward<Syncs>(syncs).t ...);
  }

  mutable std::shared_timed_mutex m;
  T t;
};
template<class T>
synchronized< T > sync( T&& t ) {
  return {std::forward<T>(t)};
}

C++14 and C++1z features included.

this assumes that const operations are multiple-reader safe (which is what std containers assume).

Use looks like:

synchronized<int> x = 7;
x.read([&](auto&& v){
  std::cout << v << '\n';
});

for an int with synchronized access.

I'd advise against having synchronized(synchronized const&). It is rarely needed.

If you need synchronized(synchronized const&), I'd be tempted to replace T t; with std::aligned_storage, allowing manual placement construction, and do manual destruction. That allows proper lifetime management.

Barring that, we could copy the source T, then read from it:

synchronized(synchronized const& o):
  t(o.read(
    [](T const&o){return o;})
  )
{}
synchronized(synchronized && o):
  t(std::move(o).read(
    [](T&&o){return std::move(o);})
  )
{}

for assignment:

synchronized& operator=(synchronized const& o) {
  access([](T& lhs, T const& rhs){
    lhs = rhs;
  }, *this, o);
  return *this;
}
synchronized& operator=(synchronized && o) {
  access([](T& lhs, T&& rhs){
    lhs = std::move(rhs);
  }, *this, std::move(o));
  return *this;
}
friend void swap(synchronized& lhs, synchronized& rhs) {
  access([](T& lhs, T& rhs){
    using std::swap;
    swap(lhs, rhs);
  }, *this, o);
}

the placement and aligned storage versions are a bit messier. Most access to t would be replaced by a member function T&t() and T const&t()const, except at construction where you'd have to jump through some hoops.

By making synchronized a wrapper instead of part of the class, all we have to ensure is that the class internally respects const as being multiple-reader, and write it in a single-threaded manner.

In the rare cases we need a synchronized instance, we jump through hoops like the above.

Apologies for any typos in the above. There are probably some.

A side benefit to the above is that n-ary arbitrary operations on synchronized objects (of the same type) work together, without having to hard-code it before hand. Add in a friend declaration and n-ary synchronized objects of multiple types might work together. I might have to move access out of being an inline friend to deal with overload conficts in that case.

live example

Answer 3

First of all, there must be something wrong with your design if you want to move an object containing a mutex.

But if you decide to do it anyway, you have to create a new mutex in move constructor, that is e.g:

// movable
struct B{};

class A {
    B b;
    std::mutex m;
public:
    A(A&& a)
        : b(std::move(a.b))
        // m is default-initialized.
    {
    }
};

This is thread-safe, because the move constructor can safely assume that its argument isn't used anywhere else, so the locking of the argument isn't required.

Answer 4

Using mutexes and C++ move semantics is an excellent way to safely and efficiently transfer data between threads.

Imagine a 'producer' thread that makes batches of strings and provides them to (one or more) consumers. Those batches could be represented by an object containing (potentially large) std::vector<std::string> objects. We absolutely want to 'move' the internal state of those vectors into their consumers without unnecessary duplication.

You simply recognize the mutex as part of the object not part of the object's state. That is, you don't want to move the mutex.

What locking you need depends on your algorithm or how generalized your objects are and what range of uses you permit.

If you only ever move from a shared state 'producer' object to a thread-local 'consuming' object you might be OK to only lock the moved from object.

If it's a more general design you will need to lock both. In such a case you need to then consider dead-locking.

If that is a potential issue then use std::lock() to acquire locks on both mutexes in a deadlock free way.

http://en.cppreference.com/w/cpp/thread/lock

As a final note you need to make sure you understand the move semantics. Recall that the moved from object is left in a valid but unknown state. It's entirely possible that a thread not performing the move has a valid reason to attempt access the moved from object when it may find that valid but unknown state.

Again my producer is just banging out strings and the consumer is taking away the whole load. In that case every time the producer tries to add to the vector it may find the vector non-empty or empty.

In short if the potential concurrent access to the moved from object amounts to a write it's likely to be OK. If it amounts to a read then think about why it's OK to read an arbitrary state.