Is Locking and UnLocking CriticalSection in Destructor to avoid crash is right way?

Question

class MyClass
{
   ...
   ...
   CCriticalSection m_Cs;

   void MyFunction()
   {
      m_Cs.Lock();

      .....

      m_Cs.Unlock(); 


   }

   MyClass::~MyClass()
   {
     ....

   }
};

I am using Critical Section in my class as above, myFunction is called by Thread 1, and when myFunction is in progress Thread 2 is deleting the object. So Unlock is crashing.

So I decided to modify MyClass destructor as below

 MyClass::~MyClass()
  {

     m_Cs.Lock();
     m_Cs.Unlock(); 
     ....

  }

This resolves my crash, because when the Thread 1 is accessing MyFunction CriticalSection is locked, so when the object is being deleted from Thread 2, the CriticalSection.Lock in Destructor is blocked until unlock is called from MyFunction. Is this right behaviour to Lock and unLock CriticalSection from destructor to avoid this crash?

Answer 1

No, this is not the correct approach, because it does not protect the scenario where thread 2 deletes the object before thread 1 calls MyFunction.

You currently have a Dangling Pointer bug in your code.

The correct solution is to modify your code to ensure that deletion of the object happens at a time when no other thread can use it. Common approaches to this include:

• Use events to signal when the threads are done using the object, the thread that does the deletion, will block waiting for everyone to say they are done.
• Use reference counting. Have every thread that uses the object "acquire" and "release" the object. The last one that releases it (e.g. when the ref count reaches 0) then have the object delete itself.