CODEWITHSUNDEEP
c++thread-safetymutexmember
myoan
myoan

Reputation: 411

C++ operator= with a mutex in class member

I have a class containing some variable, say a list and a boolean that can be written by several threads so protected with their own mutex:

class Motel
{
// [...]
private:
    list<Room> _rooms;
    boost::mutex _rooms_mtx;
    bool _opened;
    boost::mutex _opened_mtx;
}

The issue with this code is when I need a copy constructor or an operator= (even the auto-generated ones) in my case I want to put the class in a map:

boost::map<string, Motel> all_motels;
Motel grenoble(...);
all_motels["Grenoble"] = grenoble;

This is forbidden because we cannot copy a mutex:

/usr/include/boost/thread/pthread/mutex.hpp: In copy constructor ‘project::Motel::Motel(const project::Motel&)’:
/usr/include/boost/thread/pthread/mutex.hpp:33:9: error: ‘boost::mutex::mutex(const boost::mutex&)’ is private

What should I do in that case? Thank you by advance

Upvotes: 3

Views: 1568

Answers (4)

user1810087
user1810087

Reputation: 5334

You have to provide a copy constructor and an assignment overload (operator=) wich are not copying the mutexes but the data. See this running example:

#include <map>
#include <boost/thread/mutex.hpp>


class foo {
public:
    foo() {}
    foo(foo const& cp)
        : data_(cp.data_) {}

    foo& operator=(foo const& cp) {
        data_ = cp.data_;
        return *this;
    }

private:
    boost::mutex mtx_;

    bool data_;
};


int main(int argc, char* argv[]) {
    std::map<int, foo> bar;
    bar[1] = foo();
    return 0;
}

But

Mutexes are used to share objects, which is not done by copying them. So usually it is desired not to be able to copy them. Therefor you should use shared_ptr (std (c++11) or boost). Then you use a map of shared_ptr's:

int main(int argc, char* argv[]) {
    std::map<int, std::shared_ptr<foo>> bar; // untested but you see the point
    bar[1] = std::shared_ptr<foo>(new foo); 
    return 0;
}

Upvotes: 0

Luca Davanzo
Luca Davanzo

Reputation: 21528

Another solution could be use Mutex pointer.

private:
    list<Room> _rooms;
    boost::mutex* _rooms_mtx;
    bool _opened;
    boost::mutex* _opened_mtx;
}

By the way you must be sure to respect this issues:

  • will the mutex be created correctly?
  • Will the mutex be used correctly?

Upvotes: 0

vz0
vz0

Reputation: 32923

The common solution here is to make Motel noncopyable and the Motel objects allocated on the heap with new. The use of boost::noncopyable and smart pointers is also a good idea.

class Motel : private boost::noncopyable
{
// [...]
private:
    list<Room> _rooms;
    boost::mutex _rooms_mtx;
    bool _opened;
    boost::mutex _opened_mtx;
}

boost::map<string, boost::unique_ptr<Motel> > all_motels;
all_motels["Grenoble"].reset(new Motel(...));

Upvotes: 1

gnasher729
gnasher729

Reputation: 52602

You can't copy a boost::mutex (your error message shows that boost prevents it, but trying to copy a mutex is a nest of worms). At the very least you have to write your own copy constructor for Motel, and you have to figure out what to do about the mutex. Most likely you want to create a mutex yourself.

Upvotes: 0

Related Questions