bhavesh
bhavesh

Reputation: 1406

Copy Class with std::mutex

I have a class with a std::mutex as a member. I am trying to create an array of such class

class C
{
 int x;
 std::mutex m;
};

int main()
{
  C c[10];
  //later trying to create a temp C
  C temp = c[0];
}

Clearly the above is not possible as mutex object is not copyable. The way to solve it is through a copy constructor.

However, I am having problem in creating a copy constructor. I have tried

C (const C &c)
{
   x = c.x;

   //1. m
   //2. m()
   //3. m = c.m
}

I am not sure what is the right syntax out of the 3 choices. Please help.

Upvotes: 25

Views: 24084

Answers (6)

bradgonesurfing
bradgonesurfing

Reputation: 32192

All the above are bad advice and are recommending you break the rule of zero.

A better way is to create a utility class for dealing with the problem of copying mutexes according to the rule you want applied. For example ( and this may not match your requirements ) the following code

https://godbolt.org/z/Y86jscd6K

#include <iostream>
#include <variant>
#include <mutex>

struct mutex_holder {
    std::mutex mutex;
    mutex_holder():mutex(){}
    mutex_holder(mutex_holder const & other):mutex(){}
};

demonstrates a type called mutex_holder which follows the rule that upon copy the copy always gets a new mutex. You can apply this policy then to your mutex in some class you need to copy

struct A {
    mutex_holder mHolder;
    int x;
    int z;
    int y;
};

and then when you need to use the lock

void Foo(A & a)
{
    std::scoped_lock lock(a.mHolder.mutex);
    a.x = a.z + a.y;
}

And you can see that the class A is copyable and moveable without writing any custom special member function constructors.

int main(){
   A a;
   A aa = a;

   Foo(a);
   Foo(aa);

   A aaa = std::move(aa);
   Foo(aaa);
}

If you need to protect some of the members during copying you can also implement such policy classes to do this.

#include <iostream>
#include <variant>
#include <mutex>

template <typename T>
struct mutex_holder {
    std::mutex mutex;
    mutex_holder():mutex(){}
    mutex_holder(mutex_holder const & other):mutex()
    {
        std::scoped_lock lock(mutex);
        {
            data = other.data;
        }
    }
    T data;
};


struct A {
    struct Inner {
        int x;
        int z;
        int y;
    };
    mutex_holder<Inner> mHolder;
};

void Foo(A & a)
{
    std::scoped_lock lock(a.mHolder.mutex);
    a.mHolder.data.x = a.mHolder.data.z + a.mHolder.data.y;
}


int main(){
   A a;
   A aa = a;
   Foo(a);
   Foo(aa);
}

Again your business logic classes should not have custom SMF. Let the compiler write them for you.

Upvotes: 3

myaut
myaut

Reputation: 11514

You shouldn't write any of these lines. Your implementation of copy constructor is equivalent to:

C (const C &c) : x(), m()
{
   x = c.x;
}

So new instance of mutex m is default initialized which means that one of the default constructors will be called. It may be safely used.

However, there are several conserns about this code. I.e. if m protects x, you should explicitly lock it before accessing value:

C (const C &c)
{
    std::lock_guard<std::mutex> guard(c.m);
    x = c.x;
}

which would require to declare m as mutable (because c is const reference in copy ctor).

mutable std::mutex m;

In the end, you can see that copying objects with mutexes inside is confusing, and if C is public class, it'll confuse its users, so think twice before implementing copying of it.

Upvotes: 16

Sneaky Polar Bear
Sneaky Polar Bear

Reputation: 1671

As stated in other answers, there are only pretty niche situations in which you would want to do this, but in the case that you have some object class that uses a mutex internally, you will want to make copy and move constructors that explicitly declare everything to move and copy except for the mutex. This will result in the mutex (and anything else left out) being default constructed (ie each new or copied object will get its own unique mutex). Make sure that whatever your mutex is used to protect from is not being called while using the copy or move constructors as they do not (can not?) call the mutex to lock.

Here is a full example to help anyone in the future running into this issue:

class Shape
{
public:
    Shape() {} //default constructor
    Shape(double _size) //overloaded constructor
    {
        size = _size;
    }

    Shape(const Shape& obj) //copy constructor (must be explicitly declared if class has non-copyable member)
    {
        //Do not put any non-copyable member variables in here (ie the mutex), as they will be
        //default initialized if left out

        size = obj.size; //any variables you want to retain in the copy
    }
    Shape& operator=(const Shape&& obj) //move constructor (must be explicitly declared if class has non-copyable member)
    {
        //Do not put any non-copyable member variables in here (ie the mutex), as they will be
        //default initialized if left out

        size = obj.size;//any variables you want to retain in the move
        return *this;
    }

    double testMe() { return size; }
private:
    std::mutex dataMutex;
    double size;
};

Upvotes: 2

Pradheep
Pradheep

Reputation: 3819

Short answer you dont copy the mutex.

Lets start from the basics, mutex is a short name of mutual exclusion i.e you want to make sure that, when there are multiple threads you dont want them to change/modify the value in parallel. You want to serialize the access or modification/read so that the value read is valid.

In the above case you are copying a new value to the variable.In this case you need not use a mutex lock as you are creating a new object.

Upvotes: 12

Enigma
Enigma

Reputation: 1717

std::mutex m does not have to be copied. You can use the default constructed mutex.

Upvotes: 2

Massimiliano
Massimiliano

Reputation: 17000

You could use an array of shared_ptr<C>, then you won't need C itself to be copyable...

Upvotes: 7

Related Questions