11

i'm having some trouble using a C++11 std::mutex

in my class I have a variable called semaphore of type std::mutex.

So I positioned my semaphore.lock() and semaphore.unlock() before and after my critical section

class Database {
public:
Database(string, string, string, string);
virtual ~Database();

struct sMyHome getMyHome(void);
struct sPhysical getPhysical(int);
struct sSystemInfo getSystemInfo(void);
void commitSystemInfo(struct sSystemInfo);
struct sTSensors getTSensors(int);
struct sWireless getWireless(int);
struct sWirelessConf getWirelessConf(int);
struct sWirelessStatus getWirelessStatus(int);

private:
void queryMyHome(void);
void queryPhysical(int);
void querySystemInfo(void);
void queryTSensors(int ID);
void queryWireless(int ID);
void queryWirelessConf(int ID);
void queryWirelessStatus(int ID);
string toString(int);

struct sMyHome MyHome;
struct sPhysical Physical[4];
struct sSystemInfo SystemInfo;
struct sTSensors TSensors[32];
struct sWireless Wireless[64];
struct sWirelessConf WirelessConf[64];
struct sWirelessStatus WirelessStatus[64];

MYSQL *connection;
mutex semaphore;
};

this is part of the main, where I retrieve the error:

 Database db = Database("192.168.1.10", "****", "******", "******");

but the cross-compiler says

../main.cpp:8:73: error: use of deleted function 'Database::Database(const Database&)'
  Database db = Database("192.168.1.10", "root", "raspberry", "DomoHome2");
                                                                     ^
In file included from ../main.cpp:2:0:
../Database.h:79:7: note: 'Database::Database(const Database&)' is implicitly deleted      because the default definition would be ill-formed:
 class Database {
   ^
../Database.h:79:7: error: use of deleted function 'std::mutex::mutex(const std::mutex&)'
 In file included from ../Database.h:12:0,
             from ../main.cpp:2:
/Volumes/rpi-crosscompiler-toolchain/arm-none-linux-gnueabi/arm-none-linux-gnueabi/include/c++/4.8.2/mutex:128:5: error: declared here
 mutex(const mutex&) = delete;
 ^
make: *** [main.o] Error 1

and this is my constructor:

Database::Database(string Address, string Username, string Password, string Database) {
// TODO Auto-generated constructor stub
connection = mysql_init(NULL);
mysql_real_connect(connection, Address.c_str(), Username.c_str(), Password.c_str(), Database.c_str(), 0, NULL, 0);
}
Angelo Cassano
  • 180
  • 1
  • 4
  • 16
  • You are trying to copy `Database` object, but it is non-copyable. Probably because it has a `mutex` data member. I can't see why the mutex should be static BTW. – juanchopanza Jun 17 '14 at 20:23
  • You're not providing enough of your code. Your error messages are impossible to interpret unless you provide more code. –  Jun 17 '14 at 20:24
  • 1
    You aren't showing the relevant code, and that's some problem in ***your*** code ([check this one](http://stackoverflow.com/questions/12573816/what-is-an-undefined-reference-unresolved-external-symbol-error-and-how-do-i-fix)). But anyway check the various [auto lock](http://en.cppreference.com/w/cpp/thread/lock_guard) features provided from the standards to lock/unlock synchronization items. – πάντα ῥεῖ Jun 17 '14 at 20:26
  • Added more code, hope this help to clarify the situation – Angelo Cassano Jun 17 '14 at 20:36
  • 1
    Calling a `std::mutex` `semaphore` does not make it one. I would suggest you use a different name, in case some day you add an actual semaphore to your program, and someone gets completely befuddled... – twalberg Jun 17 '14 at 20:45
  • @AngeloCassano Using the current standard's classes, a _'semaphore'_ actually would be constituted using a [condition variable](http://en.cppreference.com/w/cpp/thread/condition_variable). – πάντα ῥεῖ Jun 17 '14 at 20:55

2 Answers2

22

std::mutex is neither copyable nor movable. Including one in your class necessitates that your class becomes non-copyable (-movable) as well. If you want your class to be copyable or movable, you will have to tell the compiler how objects of your class are to be copied or moved by implementing copy/move construction/assignment yourself. For example:

class synchronized_int {
  mutable std::mutex mtx;
  int value;
public:
  synchronized_int(int v = 0) : value(v) {}

  // Move initialization
  synchronized_int(synchronized_int&& other) {
    std::lock_guard<std::mutex> lock(other.mtx);
    value = std::move(other.value);
    other.value = 0;
  }

  // Copy initialization
  synchronized_int(const synchronized_int& other) {
    std::lock_guard<std::mutex> lock(other.mtx);
    value = other.value;
  }

  // Move assignment
  synchronized_int& operator = (synchronized_int&& other) {
    std::lock(mtx, other.mtx);
    std::lock_guard<std::mutex> self_lock(mtx, std::adopt_lock);
    std::lock_guard<std::mutex> other_lock(other.mtx, std::adopt_lock);
    value = std::move(other.value);
    other.value = 0;
    return *this;
  }

  // Copy assignment
  synchronized_int& operator = (const synchronized_int& other) {
    std::lock(mtx, other.mtx);
    std::lock_guard<std::mutex> self_lock(mtx, std::adopt_lock);
    std::lock_guard<std::mutex> other_lock(other.mtx, std::adopt_lock);
    value = other.value;
    return *this;
  }
};
Casey
  • 41,449
  • 7
  • 95
  • 125
  • Would you mind commenting a little on what all you're doing on the Move/Copy Assignment? I would've thought just two `lock_guards` statements on their own would be sufficient-- it looks like though you lock both of them using `lock` and then pass the locks to a `lock_guard`, presumably so they automatically unlock at end of scope. I'm just curious why you did it this way. – schrödinbug Nov 21 '16 at 13:47
  • 1
    @Jason Whenever a multithreaded program blocks waiting on mutex A while holding mutex B, it runs the risk of deadlock: if another thread holds mutex B and simultaneously blocks waiting on mutex A, neither thread will ever wake and make progress again. The standard library provides [`std::lock`](http://en.cppreference.com/w/cpp/thread/lock) as a tool to lock multiple locks while avoiding deadlock. Your analysis of the remainder of the code is correct: once `std::lock` has locked the two mutexes, I have `lock_guard`s adopt each mutex so they will properly be unlocked upon leaving the scope. – Casey Nov 21 '16 at 16:35
4

Declare your mutex dynamically and not statically.

At your Database class header declare the mutex as a pointer, like that:

mutex * semaphore;

and at your constructor initialize the mutex calling his constructor:

semaphore = new std::mutex();
Derzu
  • 7,011
  • 3
  • 57
  • 60