Designing a thread-safe copyable class

The straightforward way to make a class threadsafe is to add a mutex attribute and lock the mutex in the accessor methods

class cMyClass {
  boost::mutex myMutex;
  cSomeClass A;
public:
  cSomeClass getA() {
    boost::mutex::scoped_lock lock( myMutex );
    return A;
  }
};

The problem is that this makes the class non-copyable.

I can make things work by making the mutex a static. However, this means that every instance of the class blocks when any other instance is being accessed, because they all share the same mutex.

I wonder if there is a better way?

My conclusion is that there is no better way. Making a class thread-safe with private static mutex attribute is ‘best’: - it is simple, it works, and it hides the awkward details.

class cMyClass { static boost::mutex myMutex; cSomeClass A; public: cSomeClass getA() { boost::mutex::scoped_lock lock( myMutex ); return A; } };

The disadvantage is that all instances of the class share the same mutex and so block each other unnecessarily. This cannot be cured by making the mutex attribute non-static ( so giving each instance its own mutex ) because the complexities of copying and assignment are nightmarish, if done properly.

The individual mutexes, if required, must be managed by an external non-copyable singleton with links established to each instance when created.

Thanks for all the responses.

Several people have mentioned writing my own copy constructor and assignment operator. I tried this. The problem is that my real class has many attributes which are always changing during development. Maintaining both the copy constructor and assignmet operator is tedious and error-prone, with errors creating hard to find bugs. Letting the compiler generate these for complex class is an enormous time saver and bug reducer.

Many responses are concerned about making the copy constructor and assignment operator thread-safe. This requirement adds even more complexity to the whole thing! Luckily for me, I do not need it since all the copying is done during set-up in a single thread.

I now think that the best approach would be to build a tiny class to hold just a mutex and the critical attributes. Then I can write a small copy constructor and assignment operator for the critical class and leave the compiler to look after all the other attributes in the main class.

class cSafe {
  boost::mutex myMutex;
  cSomeClass A;
public:
  cSomeClass getA() {
    boost::mutex::scoped_lock lock( myMutex );
    return A;
  }
  (copy constructor)
  (assignment op )

};
class cMyClass {
  cSafe S;
  ( ... other attributes ... )
public:
  cSomeClass getA() {
    return S.getA();
  }
};

Solution

The simple fact is that you cannot make a class thread safe by spewing mutexes at the problem. The reason that you can't make this work is because it doesn't work, not because you're doing this technique wrong. This is what everyone noticed when multithreading first came and started slaughtering COW string implementations.

Thread design occurs at the application level, not on a per-class basis. Only specific resource management classes should have thread-safety on this level- and for them you need to write explicit copy constructors/assignment operators anyway.