Locking shared resources in constructor and destructor

Question

I believe I've got a good handle on at least the basics of multi-threading in C++, but I've never been able to get a clear answer on locking a mutex around shared resources in the constructor or the destructor. I was under the impression that you should lock in both places, but recently coworkers have disagreed. Pretend the following class is accessed by multiple threads:

class TestClass
{
public:

   TestClass(const float input) :
      mMutex(),
      mValueOne(1),
      mValueTwo("Text")
   {
      //**Does the mutex need to be locked here?
      mValueTwo.Set(input);
      mValueOne = mValueTwo.Get();
   }

   ~TestClass() 
   { 
     //Lock Here?
   }

   int GetValueOne() const
   {
      Lock(mMutex);
      return mValueOne;
   }

   void SetValueOne(const int value)
   {
      Lock(mMutex);
      mValueOne = value;
   }

   CustomType GetValueTwo() const
   {
      Lock(mMutex);
      return mValueOne;
   }

   void SetValueTwo(const CustomType type)
   {
      Lock(mMutex);
      mValueTwo = type;
   }

private:

   Mutex mMutex;
   int mValueOne;
   CustomType mValueTwo;
};

Of course everything should be safe through the initialization list, but what about the statements inside the constructor? In the destructor would it be beneficial to do a non-scoped lock, and never unlock (essentially just call pthread_mutex_destroy)?

Answer 1

Multiple threads cannot construct the same object, nor should any thread be allowed to use the object before it's fully constructed. So, in sane code, construction without locking is safe.

Destruction is a slightly harder case. But again, proper lifetime management of your object can ensure that an object is never destroyed when there's a chance that some thread(s) might still use it.

A shared pointer can help in achieving this eg. :

• construct the object in a certain thread
• pass shared pointers to every thread that needs access to the object (including the thread that constructed it if needed)
• the object will be destroyed when all threads have released the shared pointer

But obviously, other valid approaches exist. The key is to keep proper boundaries between the three main stages of an object's lifetime : construction, usage and destruction. Never allow an overlap between any of these stages.

Answer 2

They don't have to be locked in the constructor, as the only way anyone external can get access to that data at that point is if you pass them around from the constructor itself (or do some undefined behaviour, like calling a virtual method).

[Edit: Removed part about destructor, since as a comment rightfully asserts, you have bigger issues if you're trying to access resources from an object which might be dead]