Thread-Safe implementation of an object that deletes itself

Question

I have an object that is called from two different threads and after it was called by both it destroys itself by "delete this".

How do I implement this thread-safe? Thread-safe means that the object never destroys itself exactly one time (it must destroys itself after the second callback).

I created some example code:

class IThreadCallBack                                                                                             
{                                                                                                                 
  virtual void CallBack(int) = 0;                                                                                 
};                                                                                                                

class M: public IThreadCallBack                                                                                   
{                                                                                                                 
private:                                                                                                          
  bool t1_finished, t2_finished;                                                                                  

public:                                                                                                           

  M(): t1_finished(false), t2_finished(false)                                                                     
  {                                                                                                               
    startMyThread(this, 1);                                                                                       
    startMyThread(this, 2);                                                                                       
  }                                                                                                               

  void CallBack(int id)                                                                                           
  {                                                                                                               
    if (id == 1)                                                                                                  
    {                                                                                                             
      t1_finished = true;                                                                                         
    }                                                                                                             
    else                                                                                                          
    {                                                                                                             
      t2_finished = true;                                                                                         
    }                                                                                                             

    if (t1_finished && t2_finished)                                                                               
    {                                                                                                             
      delete this;                                                                                                
    }                                                                                                             
  }                                                                                                               
};                                                                                                                

int main(int argc, char **argv) {                                                                                 
  M* MObj = new M();                                                                                              
  while(true);                                                                                                    
}                                                                                                                 

Obviously I can't use a Mutex as member of the object and lock the delete, because this would also delete the Mutex. On the other hand, if I set a "toBeDeleted"-flag inside a mutex-protected area, where the finised-flag is set, I feel unsure if there are situations possible where the object isnt deleted at all. Note that the thread-implementation makes sure that the callback method is called exactly one time per thread in any case.

Edit / Update: What if I change Callback(..) to:

void CallBack(int id)

{    
  mMutex.Obtain()
  if (id == 1)
  {
    t1_finished = true;
  }
  else
  {
    t2_finished = true;
  }
  bool both_finished = (t1_finished && t2_finished);

  mMutex.Release();

  if (both_finished)
  {
    delete this;     
  }
}

Can this considered to be safe? (with mMutex being a member of the m class?)

I think it is, if I don't access any member after releasing the mutex?!

Answer 1

Instead of using two separate flags, you could consider setting a counter to the number of threads that you're waiting on and then using interlocked decrement.

Then you can be 100% sure that when the thread counter reaches 0, you're done and should clean up.

For more info on interlocked decrement on Windows, on Linux, and on Mac.

Answer 2

The more robust implementation would be to implement reference counting. For each thread you start, increase a counter; for each callback call decrease the counter and if the counter has reached zero, delete the object. You can lock the counter access, or you could use the Interlocked class to protect the counter access, though in that case you need to be careful with potential race between the first thread finishing and the second starting.

Update: And of course, I completely ignored the fact that this is C++. :-) You should use InterlockExchange to update the counter instead of the C# Interlocked class.

Answer 3

I once implemented something like this that avoided the ickiness and confusion of delete this entirely, by operating in the following way:

• Start a thread that is responsible for deleting these sorts of shared objects, which waits on a condition
• When the shared object is no longer being used, instead of deleting itself, have it insert itself into a thread-safe queue and signal the condition that the deleter thread is waiting on
• When the deleter thread wakes up, it deletes everything in the queue

If your program has an event loop, you can avoid the creation of a separate thread for this by creating an event type that means "delete unused shared objects" and have some persistent object respond to this event in the same way that the deleter thread would in the above example.

Answer 4

I can't imagine that this is possible, especially within the class itself. The problem is two fold:

1) There's no way to notify the outside world not to call the object so the outside world has to be responsible for setting the pointer to 0 after calling "CallBack" iff the pointer was deleted.

2) Once two threads enter this function you are, and forgive my french, absolutely fucked. Calling a function on a deleted object is UB, just imagine what deleting an object while someone is in it results in.

I've never seen "delete this" as anything but an abomination. Doesn't mean it isn't sometimes, on VERY rare conditions, necessary. Problem is that people do it way too much and don't think about the consequences of such a design.

I don't think "to be deleted" is going to work well. It might work for two threads, but what about three? You can't protect the part of code that calls delete because you're deleting the protection (as you state) and because of the UB you'll inevitably cause. So the first goes through, sets the flag and aborts....which of the rest is going to call delete on the way out?

Answer 5

Use Boost's Smart Pointer. It handles this automatically; your object won't have to delete itself, and it is thread safe.

Edit:
From the code you've posted above, I can't really say, need more info. But you could do it like this: each thread has a shared_ptr object and when the callback is called, you call shared_ptr::reset(). The last reset will delete M. Each shared_ptr could be stored with thread local storeage in each thread. So in essence, each thread is responsible for its own shared_ptr.