Reputation: 1406
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
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
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
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
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
Reputation: 1717
std::mutex m does not have to be copied. You can use the default constructed mutex.
Upvotes: 2
Reputation: 17000
You could use an array of shared_ptr<C>
, then you won't need C
itself to be copyable...
Upvotes: 7