3
votes

I have a class with deleted copy constructors and I'm trying to put a mutex member in something like this:

struct A {
    A(const A &other) = delete;
    A& operator=(const A &other) = delete;
    A(A&& other) = default;
    A& operator=(A &&other) = default;

    std::mutex lock;
};

The compiler is complaining that I'm trying to call the deleted copy constructor, which I summarise to be due to the std::mutex type being non-movable. How can I make the mutex member play with the move constructors with minimal fuss? I don't actually want to move the mutex member itself into the newly constructed object, and would actually like each moved object to just construct it's own mutex

3
This might be interesting: SO: Move constructor for std::mutex. - Scheff's Cat
I'm not sure whether a default move constructor/assignment is a good idea. Shouldn't the lock being responsible to guard something? Shouldn't the move construction/assigment consider that lock of the moved object? - Scheff's Cat

3 Answers

5
votes

I don't actually want to move the mutex member itself into the newly constructed object, and would actually like each moved object to just construct it's own mutex

Then simply define your move constructor to construct a new mutex:

struct A {
    A(const A &other) = delete;
    A& operator=(const A &other) = delete;
    A(A&& other)
        : lock()
    {
    }

    A& operator=(A &&other) = delete;

    std::mutex lock;
};

Move assignment will still be a problem and should probably just be deleted. Unless you can answer the question: what happens to the existing mutex member when you're being assigned a new value? Particularly: what if you are assigned a new value while the existing mutex is locked?

5
votes

As an alternative to providing custom move operations for your class, you could create a generic wrapper:

template <class T>
class PerObject
{
  T data;
public:
  PerObject() = default;
  PerObject(const PerObject&) {}
  PerObject& operator= (const PerObject&) { return *this; }
  T& operator* () const { return data; }
  T* operator-> () const { return &data; }
};

And use it like this:

struct A {
    A(const A &other) = delete;
    A& operator=(const A &other) = delete;
    A(A&& other) = default;
    A& operator=(A &&other) = default;

    PerObject<std::mutex> lock;
};

The wrapper's copy (and move) operations are no-ops, so the object containing the wrapper will always contain the one it started with.


Caveat: However, based on how your class uses the mutex, the above could actually be dangerous. If the mutex is used to protect other data within the class, then it should likely be locked while the object is being assigned to, so you will have to provide manual move operations anyway. In such case, the code would likely look something like this:

struct A {
    A(A&& other) : lock{}, /* anything else you need to move-construct */
    {
      // Note: it might even be necessary to lock `other.lock` before moving from it, depending on your class's exact semantics and expected use.
    }
    A& operator=(A &&other)
    {
      if (this == &other) return *this;  // Otherwise, double-locking would be possible

      // If you need to lock only this object:
      std::unique_lock<std::mutex> l(lock);
      // Alternatively, if you need to lock both objects:
      std::scoped_lock l(lock, other.lock);

      // Now move data from other to this

      return *this;
    }

    std::mutex lock;
};
1
votes

One way is to make your move constructor to create new mutex when called.

 A(A&& other): lock()
 {
     //... move other things
 }

You can also use a std::unique_ptr() to the std::mutex since it is movable.

struct A {
    A(const A &other) = delete;
    A& operator=(const A &other) = delete;
    A(A&& other) = default;
    A& operator=(A &&other) = default;

    std::unique_ptr<std::mutex> lock;
};

A::A() : lock(new std::mutex())

With this approach you'll not create new mutex each time you move the object which will remove some of the overhead.