CODEWITHSUNDEEP
c++multithreadingdestructor
aJ.
aJ.

Reputation: 35450

Should destructors be threadsafe?

I was going through a legacy code and found the following snippet:

MyClass::~MyClass()
{
   EnterCriticalSection(&cs);

//Access Data Members, **NO Global** members are being accessed here


  LeaveCriticalSection(&cs);
}

I am wondering will it help by any chance to guard the destructor ?

Consider a scenario :

1. Thread1 - About to execute any of the member function which uses critical section
2. Thread2-  About to execute destructor.

If the order of execution is 1=>2 then it might work. But what if the order is reversed ?

Is it a design issue ?

Upvotes: 17

Views: 9646

Answers (9)

lumpidu
lumpidu

Reputation: 759

Old question, but still valid IMHO.

In general, public members of a class altering a critical section, accessible from different threads should lock this critical section. But destruction of an object is the ultimate alteration of the state of an object including the critical section.

So if there are async operations going on, where this critical state of the object is entered, destruction should definitely wait until that critical section is left again. One way to do it, is to use locking in the destructor. Surely that doesn't help to guarantee the object itself isn't accessed erroneously anymore afterwards.

But that technique can be used to sync. destruction of the object with ending of the async. operation on the critical section.

Upvotes: 0

Matthew
Matthew

Reputation: 1

Your comments says "NO Global members are being accessed here" so I'd guess not. Only the thread that created an object should destroy it, so from what other thread would you be protecting it?

I like orderly creation and destruction myself, where only a single object ever owns another sub-object, and any other object with a reference to that sub-object is a descendant further down in the tree. If any of those sub-objects represent different threads, then they will have made sure to complete before destruction proceeds up the tree.

Example:

  • main() create object A
    • object A contains object B
      • object B contains object C
        • object C creates a thread that accesses objects A and B
        • object C's destructor runs, waiting for its thread to finish
      • object B's destructor runs
    • object A's destructor runs
  • main() returns

The destructors for object A and B don't need to think about threads at all, and object C's destructor only needs to implement some communication mechanism (waiting on an event, for instance) with the thread it chose to create itself.

You can only get into trouble if you start handing out references (pointers) to your objects to arbitrary threads without keeping track of when those threads are created and destroyed, but if you're doing that then you should be using reference counting, and if you are then it's too late by the time the destructor is called. If there's still a reference to an object, then no one should have even tried to invoke its destructor.

Upvotes: 0

David Rodríguez - dribeas
David Rodríguez - dribeas

Reputation: 208323

I have seen a case with ACE threads, where the thread is running on an ACE_Task_Base object and the object is destroyed from another thread. The destructor acquires a lock and notifies the contained thread, just before waiting on a condition. The thread that is running on the ACE_Task_Base signal signals the condition at exit and lets the destructor complete and the first thread exit:

class PeriodicThread : public ACE_Task_Base
{
public:
   PeriodicThread() : exit_( false ), mutex_()
   {
   }
   ~PeriodicThread()
   {
      mutex_.acquire();
      exit_ = true;
      mutex_.release();
      wait(); // wait for running thread to exit
   }
   int svc()
   {
      mutex_.acquire();
      while ( !exit_ ) { 
         mutex_.release();
         // perform periodic operation
         mutex_.acquire();
      }
      mutex_.release();
   }
private:
   bool exit_;
   ACE_Thread_Mutex mutex_;
};

In this code, the destructor must use thread safety techniques to guarantee that the object is not destroyed before the thread that is running svc() exits.

Upvotes: 5

Mehrdad Afshari
Mehrdad Afshari

Reputation: 421968

The destructor should not be called when the object is in use. If you're dealing with such a situation, it needs a fundamental fix. However, the destructor might want to alter some other thing (which is unrelated to the class being destructed) and it might need a critical section (e.g. like decrementing a global counter).

Upvotes: 37

Vijay Angelo
Vijay Angelo

Reputation: 762

I second the comment from Neil ButterWorth. Absolutely, the Entities responsible for deleting and accessing the myclass, should have a check on this.

This synchronisation will start actually from the moment the object of type MyClass is created.

Upvotes: 0

anon
anon

Reputation:

Define "thread safe". These are possibly the two most ill-understood words in modern computing.

But if there is a possibility of the destructor being entered twice from two different threads (as the use of symchronisation objects implies) your code is in deep doo-doo. The objects that are deleting the object that you are asking about should be managing this - it is (probably) at that level that synchronisation should be taking place.

Upvotes: 3

Jimmy J
Jimmy J

Reputation: 1985

If you're accessing global variables you might need thread safety, yes

eg. My "Window" class adds itself to the list "knownWindows" in the constructor and removes itself in the destructor. "knownWindows" needs to be threadsafe so they both lock a mutex while they do it.

On the other hand, if your destructor only accesses members of the object being destroyed, you have a design issue.

Upvotes: 4

nobody
nobody

Reputation: 20163

Not going to make a difference. If, as you say, the order of the calls is reversed then you're calling a member function on a destructed object and that's going to fail. Synchronization can't fix that logical error (for starters, the the member function call would be trying to acquire a lock object that's been destructed).

Upvotes: 0

JaredPar
JaredPar

Reputation: 754505

I think you have a more fundamental problem. It shouldn't be legal to destroy your object on one thread while another thread is still calling member functions. This in itself is wrong.

Even if you successfully guard your destructor with critical sections, what happens when the other thread starts executing the remainder of the function? It will be doing so on a deleted object which (depending on it's allocation location) will be garbage memory or simple an invalid object.

You need to alter your code to ensure the object is not destructed while still in use.

Upvotes: 5

Related Questions