Error C2280 mutex in a class C++

Question

I'm having an issue with declaring a mutex in a class, whenever I attempt instantiate the class the Error C2280 appears. My project is too create a simulation of the Dining Philosopher's Problem.

Declaration:

class Fork
{

public:

struct _position
{
    float x;
    float y;
};

Fork();
Fork(int num_phil_, int new_index_);
_position ReturnWorkPosition(){ _position return_position_; /*position_mutex_.lock();*/ return_position_ = work_position_; 
                                    /*position_mutex_.unlock();*/ return return_position_; };
float ReturnRotation(){ return rotation_; };

void CalculateWorkPosition(int master_index_);
void ResetWorkPosition(){ /*position_mutex_.lock();*/ work_position_ = orig_position_; /*position_mutex_.unlock();*/ };
void TakeOwnership(int master_index_);
void RelinquishOwnership();

private:

int index_;
int num_philosophers_;
float rotation_;

_position orig_position_;
_position work_position_;

//std::mutex master_mutex_;
//std::mutex position_mutex_;
};

Instantiation:

for (int i = 0; i < num_philosophers_; i++)
{
    Fork temp_fork_(num_philosophers_, i);
    fork_.push_back(temp_fork_);
}

Edit:: New Declaration:

class Fork
{

public:

Fork();
Fork(int num_phil_, int new_index_);

Fork(Fork const&) = delete;
Fork& operator=(Fork const&) = delete;

struct _position
{
    float x;
    float y;
};

_position ReturnWorkPosition(){ _position return_position_; /*position_mutex_.lock();*/ return_position_ = work_position_; 
                                    /*position_mutex_.unlock();*/ return return_position_; };
float ReturnRotation(){ return rotation_; };

void CalculateWorkPosition(int master_index_);
void ResetWorkPosition(){ /*position_mutex_.lock();*/ work_position_ = orig_position_; /*position_mutex_.unlock();*/ };
void TakeOwnership(int master_index_);
void RelinquishOwnership();

private:

int index_;
int num_philosophers_;
float rotation_;

_position orig_position_;
_position work_position_;

//std::mutex master_mutex_;
//std::mutex position_mutex_;
};

Error messages:

Error   18  error C2280: 'Fork &Fork::operator =(const Fork &)' : attempting to reference a deleted function    c:\program files (x86)\microsoft visual studio 12.0\vc\include\xutility 2045    1   Lab3e

Error   13  error C2280: 'Fork::Fork(const Fork &)' : attempting to reference a deleted function    c:\program files (x86)\microsoft visual studio 12.0\vc\include\xmemory0 600 1   Lab3e

New Instantiation:

for (int i = 0; i < num_philosophers_; i++)
{
    Fork temp_fork_(num_philosophers_, i);
    fork_.emplace_back(temp_fork_);
}

Answer 1

You need to delete your copy constructor and copy-assignment operator. You cannot copy a std::mutex and therefore you cannot copy an instance of your class, you can only construct or move one.

Fork(Fork const&) = delete;
Fork& operator=(Fork const&) = delete;

The error is telling you that you are trying to reference a deleted function, which is the copy constructor, because push_back is trying to copy your temp_fork_. Instead, I would use emplace_back to create a Fork in-place.

for (int i = 0; i < num_philosophers_; i++)
{
    fork_.emplace_back(num_philosophers_, i);
}

Or

for (int i = 0; i < num_philosophers_; i++)
{
    Fork temp_fork_(num_philosophers_, i);
    fork_.push_back(std::move(temp_fork_));
}

Side note
Instead of manually locking and unlocking your std::mutex, you can do so using a std::lock_guard, which is a class set up to handle locking/unlocking a mutex via RAII

void ResetWorkPosition()
{
    std::lock_guard<std::mutex> lock(position_mutex_);  // position_mutex_ is now locked
    work_position_ = orig_position_;
};  // lock fell out of scope, position_mutex_ is now unlocked

_position ReturnWorkPosition()
{
    _position return_position_;
    std::lock_guard<std::mutex> lock(position_mutex_);  // position_mutex_ is now locked
    return_position_ = work_position_;
    return return_position_;
};

Answer 2

As @sbabbi pointed out in the comments to @CoryKramer's answer, essentially, the problem is that your class needs to have a copy or move constructor and because some of its members (the mutexes) don't have one of their own, the compiler implicitly deletes (my term) those special functions for Fork. So you have to implement at least one of them on your own for Fork or use pointers (see below).

Explanation

In order to be storable in a container, your class T has to fulfill certain requirements. For a std::vector and until c++11, T had to be CopyAssignable and CopyConstructible. Since c++11 however, the exact requirements depend on the functions you are actually trying to call (T always has to be erasable) .

• For push_back it has to be CopyInsertable or - if the argument is an rvalue (e.g. a temporary) - MoveInsertable (I believe that any CopyInsertable type is also MoveInsertable)
• For emplace_backit has to be MoveInsertable and EmplaceConstructible.

Essentially, this means, your type has to have a copy constructor when you want to use push_back and even if you are using emplace_back, you still need at least a move constructor. The rational is that if a push/emplace_back causes a re-allocation, the elements have to be moved from the old to the new memory region.

Now copying or moving a mutex doesn't make much sense, which is why it doesn't have any of those member functions (they are declared as deleted by the standard). As a result, your compiler also doesn't implicitly define those copy/move constructors for your Fork class, but rather implicitly deletes them and then gives you the error that you are trying to use a deleted function.

Possible solutions

You can define your own copy constructor, which would look something like this:

Fork::Fork(const Fork& other) {
    /* potentially locking a mutex of other, 
       which would have to be declared mutable */
    index_            =other.index_;
    num_philosophers_ =other.num_philosophers_;
    rotation_         =other.rotation_;
    orig_position_    =other.orig_position_;
    work_position_    =other.work_position_;
}

The problem is that it might be hard to define the correct semantics of a copy constructor in the context of your (multi-threaded) program. If you only need it for inserting Forks into a vector, that isn't modified during the further execution of your program, you should be fine with just ignoring the mutexes though.

However, it is probably better to store a pointer to a Fork instead:

for (int i = 0; i < num_philosophers_; i++) {      
    fork_.push_back(std::unique_ptr<Fork>(new Fork(num_philosophers_, i)));
    //or if your compiler is c++14 compatible:
    fork_.push_back(std::make_unique<Fork>(num_philosophers_, i));      
}

Thirdly, you can use a different container like std::list instead of vector (you'd still have to use emplace_back instead of push_back though).