What Happens When Calling the Destructor of a Thread Object That Has a Condition Variable Waiting

What happens when calling the destructor of a thread object that has a condition variable waiting?

From the C++11 spec:

30.3.1.3 thread destructor [thread.thread.destr] ~thread();

If joinable(), calls std::terminate(). Otherwise, has no effects.

[ Note: Either implicitly detaching or joining a joinable() thread in its destructor could result in difficult to debug correctness (for detach) or performance (for join) bugs encountered only when an exception is raised. Thus the pro grammer must ensure that the destructor is never executed while the thread is still joinable. — end note ]

So calling a thread destructor without first calling join (to wait for it to finish) or detach is guarenteed to immediately call std::terminate and end the program.

Destruction of condition variable randomly loses notification

I am pretty sure your vendors implementation is broken. Your program looks almost OK from the perspective of obeying the contract with the cv/mutex classes. I couldn’t 100% verify, I am behind one version.

The notion of “blocking” is confusing in the condition_variable (CV) class because there are multiple things to be blocking on. The contract requires the implementation to be more complex than a veneer on pthread_cond* (for example). My reading of it indicates that a single CV would require at least 2 pthread_cond_t’s to implement.

The crux is the destructor having a definition while threads are waiting upon a CV; and its ruin is in a race between CV.wait and ~CV. The naive implementation simply has ~CV broadcast the condvar then eliminate it, and has CV.wait remember the lock in a local variable, so that when it awakens from the runtime notion of blocking it no longer has to reference the object. In that implementation, ~CV becomes a “fire and forget” mechanism.

Sadly, a racing CV.wait could meet the preconditions, yet not be finished interacting with the object yet, when ~CV sneaks in and destroys it. To resolve the race CV.wait and ~CV need to exclude each other, thus the CV requires at least a private mutex to resolve races.

We aren’t finished yet. There usually isn’t an underlying support [ eg. kernel ] for an operation like “wait on cv controlled by lock and release this other lock once I am blocked”. I think that even the posix folks found that too funny to require. Thus, burying a mutex in my CV isn’t enough, I actually require a mechanism that permits me to process events within it; thus a private condvar is required inside the implementation of CV. Obligatory David Parnas meme.

Almost OK, because as Marek R points out, you are relying on referencing a class after its destruction has begun; not the cv/mutex class, your notify_on_delete class. The conflict is a bit academic. I doubt clang would depend upon nod remaining valid after control had transferred to nod->cv.wait(); but the real customer of most compiler vendors are benchmarks, not programmers.

As as general note, multi-threaded programming is difficult, and having now peaked at the c++ threading model, it might be best to give it a decade or two to settle down. It’s contracts are astonishing. When I first looked at your program, I thought ‘duh, there is no way you can destroy a cv that can be accessed because RAII’. Silly me.

Pthreads is another awful API for threading. At least it doesn’t attempt over-reach, and is mature enough that robust test suites keep vendors in line.

Destruction of class with condition variable in wait

Suppose we are calling ~myclass from the master thread (call it Thread A), you can check whether it is needed to be stopped in the slave thread (call it Thread B):

And we have an Atomic<bool> variable named Stop which is accessible from both Threads.

In the thread A:

~myclass()
{
    Stop = true;   
    myConditionVariable.notifyAll();
    myThread.join();
}

In the thread B:

myConditionVariable.wait(myMutexLock, [&] return Stop || haveWorkToDo(); );
if (Stop)
   // Terminate Thread
// continue to work

Destroying condition_variable while waiting

You can delete it right away.

Quote from C++ standard:

~ condition_variable();

Requires: There shall be no thread blocked on *this. [Note: That is,
all threads shall have been notified; they may subsequently block on
the lock specified in the wait. This relaxes the usual rules, which
would have required all wait calls to happen before destruction. Only
the notification to unblock the wait must happen before destruction.

Basically wait functions are required to perform locking and waiting atomically:

The execution of notify_one and notify_all shall be atomic. The execution
of wait, wait_for, and wait_until shall be performed in three atomic parts:

1. the release of the mutex and entry into the waiting state;
2. the unblocking of the wait; and
3. the reacquisition of the lock.

Once notify wakes a thread, it should be considered "unblocked" and should contest the mutex.

---

There are similar guarantees about std::mutex: threads are not required to leave unlock before mutex is destroyed.

Quote from C++ standard:

The implementation shall provide lock and unlock operations, as
described below. For purposes of determining the existence of a data
race, these behave as atomic operations. The lock and unlock
operations on a single mutex shall appear to occur in a single total
order.

Later:

Note: After a thread A has called unlock(), releasing a mutex, it is
possible for another thread B to lock the same mutex, observe that it
is no longer in use, unlock it, and destroy it, before thread A
appears to have returned from its unlock call.

Such guarantees are required to avoid issues like this, when mutex inside an object is used to protect object reference counter.

---

Note that this does not guarantee that your implementation has no bugs in this regard. In the past glibc had multiple bugs related to the destruction of synchronization objects, in particular pthread_mutex_unlock was accessing mutex before returning.

Destructor, when object's dynamic variable is locked by mutex will not free it?

1. Is is it possible that var will not be deleted in destructor?

With

~Object()
{
    mu.lock();
    delete[]var; // destructor should free all dynamic memory on it's own, as I remember
    mu.unlock();
}

You might have to wait that lock finish, but var would be deleted.

Except that your program exhibits undefined behaviour with non protected concurrent access to object. (delete object isn't protected, and you read it in your another thread), so everything can happen.

2. Do I use mutex with detached threads correctly in code above?

Detached or not is irrelevant.

And your usage of mutex is wrong/incomplete.
which variable should your mutex be protecting?

It seems to be a mix between object and var.
If it is var, you might reduce scope in do_something (lock only in if-block)

And it misses currently some protection to object.

2.1 Do I need cover by mutex::lock and mutex::unlock also delete object line?

Yes object need protection.

But you cannot use that same mutex for that. std::mutex doesn't allow to lock twice in same thread (a protected delete[]var; inside a protected delete object) (std::recursive_mutex allows that).

So we come back to the question which variable does the mutex protect?

if only object (which is enough in your sample), it would be something like:

#include <thread>
#include <mutex>
using namespace std;

mutex mu;

class Object
{
public:
  char *var;

  Object()
  {
      var = new char[1]; var[0] = 1;
  }
  ~Object()
  {
      delete[]var; // destructor should free all dynamic memory on it's own, as I remember
  }
}*object = nullptr;

void do_something()
{
    for(;;)
    {
         mu.lock();
         
         if(object)
             if(object->var[0] < 255)
                  object->var[0]++;
             else
                  object->var[0] = 0;

         mu.unlock();
    }
}

int main()
{
   object = new Object();

   thread th(do_something);
   th.detach();

   Sleep(1000);

   mu.lock(); // or const std::lock_guard<std::mutex> lock(mu); and get rid of unlock
   delete object;
   object = nullptr;
   mu.unlock();

   return 0;
}

Alternatively, as you don't have to share data between thread, you might do:

int main()
{
   Object object;

   thread th(do_something);

   Sleep(1000);

   th.join();
   return 0;
}

and get rid of all mutex

How to reattach thread or wait for thread completion before exiting

Assuming that the IO thread is coded by you, you can handle this with a combination of std::promise and std::future, something like this:

#include <chrono>
#include <thread>
#include <future>
#include <iostream>

using namespace std::chrono_literals;

void demo_thread (std::promise <bool> *p)
{
    std::cout << "demo thread waiting...\n";
    std::this_thread::sleep_for (1000ms);
    std::cout << "demo thread terminating\n";
    p->set_value (true);
}

int main ()
{
    std::promise <bool> p;
    std::thread t = std::thread (demo_thread, &p);
    t.detach ();

    // ...

    std::cout << "main thread waiting...\n";
    std::future <bool> f = p.get_future();
    f.wait ();

    std::cout << "main thread terminating\n";
}

Live demo

Is it a good idea to shut down a class's thread member in the class's destructor?

It is a good idea to release resources a class creates when the class is destroyed, even if one of the resources is a thread. If the resource is created explicitly via a user call, such as Worker::Start(), then there should also be an explicit way to release it, such as Worker::Stop(). It would also be a good idea to either perform cleanup in the destructor in the event that the user does not call Worker::Stop() and/or provide the user a scoped helper class that implements the RAII-idiom, invoking Worker::Start() in its constructor and Worker::Stop() in its destructor. However, if resource allocation is done implicitly, such as in the Worker constructor, then the release of the resource should also be implicit, leaving the destructor as the prime candidate for this responsibility.

---

Destruction

Lets examine Worker::~Worker(). A general rule is to not throw exceptions in destructors. If a Worker object is on a stack that is unwinding from another exception, and Worker::~Worker() throws an exception, then std::terminate() will be invoked, killing the application. While Worker::~Worker() is not explicitly throwing an exception, it is important to consider that some of the functions it is invoking may throw:

• m_Condition.notify_one() does not throw.
• m_pThread->join() could throw boost::thread_interrupted.

If std::terminate() is the desired behavior, then no change is required. However, if std::terminate() is not desired, then catch boost::thread_interrupted and suppress it.

Worker::~Worker()
{
  m_Running = false;
  m_Condition.notify_one();
  try
  {
    m_pThread->join();
  }
  catch ( const boost::thread_interrupted& )
  {
    /* suppressed */ 
  }
}

---

Concurrency

Managing threading can be tricky. It is important to define the exact desired behavior of functions like Worker::Wake(), as well as understand the behavior of the types that facilitate threading and synchronization. For example, boost::condition_variable::notify_one() has no effect if no threads are blocked in boost::condition_variable::wait(). Lets examine the possible concurrent paths for Worker::Wake().

Below is a crude attempt to diagram concurrency for two scenarios:

• Order-of-operation occurs from top-to-bottom. (i.e. Operations at the top occur before operations at the bottom.
• Concurrent operations are written on the same line.
• < and > are used to highlight when one thread is waking up or unblocking another thread. For example A > B indicates that thread A is unblocking thread B.

Scenario: Worker::Wake() invoked while Worker::ThreadProc() is blocked on m_Condition.

Other Thread                       | Worker::ThreadProc
-----------------------------------+------------------------------------------
                                   | lock( m_Mutex )
                                   | `-- m_Mutex.lock()
                                   | m_Condition::wait( lock )
                                   | |-- m_Mutex.unlock()
                                   | |-- waits on notification
Worker::Wake()                     | |
|-- lock( m_Mutex )                | |
|   `-- m_Mutex.lock()             | |
|-- m_Condition::notify_one()      > |-- wakes up from notification
`-- ~lock()                        | `-- m_Mutex.lock() // blocks
    `-- m_Mutex.unlock()           >     `-- // acquires lock
                                   | // do some work here
                                   | ~lock() // end of for loop's scope
                                   | `-- m_Mutex.unlock()

Result: Worker::Wake() returns fairly quickly, and Worker::ThreadProc runs.

---

Scenario: Worker::Wake() invoked while Worker::ThreadProc() is not blocked on m_Condition.

Other Thread                       | Worker::ThreadProc
-----------------------------------+------------------------------------------
                                   | lock( m_Mutex )
                                   | `-- m_Mutex.lock()
                                   | m_Condition::wait( lock )
                                   | |-- m_Mutex.unlock()
Worker::Wake()                     > |-- wakes up
                                   | `-- m_Mutex.lock()
Worker::Wake()                     | // do some work here
|-- lock( m_Mutex )                | // still doing work...
|   |-- m_Mutex.lock() // block    | // hope we do not block on a system call
|   |                              | // and more work...
|   |                              | ~lock() // end of for loop's scope
|   |-- // still blocked           < `-- m_Mutex.unlock()
|   `-- // acquires lock           | lock( m_Mutex ) // next 'for' iteration.
|-- m_Condition::notify_one()      | `-- m_Mutex.lock() // blocked
`-- ~lock()                        |     |-- // still blocked
    `-- m_Mutex.unlock()           >     `-- // acquires lock
                                   | m_Condition::wait( lock )    
                                   | |-- m_Mutex.unlock()
                                   | `-- waits on notification
                                   |     `-- still waiting...

Result: Worker::Wake() blocked as Worker::ThreadProc did work, but was a no-op, as it sent a notification to m_Condition when no one was waiting on it.

This is not particularly dangerous for Worker::Wake(), but it can cause problems in Worker::~Worker(). If Worker::~Worker() runs while Worker::ThreadProc is doing work, then Worker::~Worker() may block indefinitely when joining the thread, as the thread may not be waiting on m_Condition at the point in which it is notified, and Worker::ThreadProc only checks m_Running after it is done waiting on m_Condition.

---

Working Towards a Solution

In this example, lets define the following requirements:

• Worker::~Worker() will not cause std::terminate() to be invoked.
• Worker::Wake() will not block while Worker::ThreadProc is doing work.
• If Worker::Wake() is called while Worker::ThreadProc is not doing work, then it will notify Worker::ThreadProc to do work.
• If Worker::Wake() is called while Worker::ThreadProc is doing work, then it will notify Worker::ThreadProc to perform another iteration of work.
• Multiple calls to Worker::Wake() while Worker::ThreadProc is doing work will result in Worker::ThreadProc performing a single additional iteration of work.

Code:

#include <boost/thread.hpp>
 
class Worker
{
public:
  Worker();
  ~Worker();
  void Wake();
private:
  Worker(Worker const& rhs);             // prevent copying
  Worker& operator=(Worker const& rhs);  // prevent assignment
  void ThreadProc();
 
  enum state { HAS_WORK, NO_WORK, SHUTDOWN };
  
  state                            m_State;
  boost::mutex                     m_Mutex;
  boost::condition_variable        m_Condition;
  boost::thread                    m_Thread;
};
 
Worker::Worker()
  : m_State(NO_WORK)
  , m_Mutex()
  , m_Condition()
  , m_Thread()
{
  m_Thread = boost::thread(&Worker::ThreadProc, this);
}
 
Worker::~Worker()
{
  // Create scope so that the mutex is only locked when changing state and
  // notifying the condition.  It would result in a deadlock if the lock was
  // still held by this function when trying to join the thread.
  {
    boost::lock_guard<boost::mutex> lock(m_Mutex);
    m_State = SHUTDOWN;
    m_Condition.notify_one();
  }
  try { m_Thread.join(); }
  catch ( const boost::thread_interrupted& ) { /* suppress */ };
}
 
void Worker::Wake()
{
  boost::lock_guard<boost::mutex> lock(m_Mutex);
  m_State = HAS_WORK;
  m_Condition.notify_one();
}
 
void Worker::ThreadProc()
{
  for (;;)
  {
    // Create scope to only lock the mutex when checking for the state.  Do
    // not continue to hold the mutex wile doing busy work.
    {
      boost::unique_lock<boost::mutex> lock(m_Mutex);
      // While there is no work (implies not shutting down), then wait on
      // the condition.
      while (NO_WORK == m_State)
      {
        m_Condition.wait(lock);
        // Will wake up from either Wake() or ~Worker() signaling the condition
        // variable.  At that point, m_State will either be HAS_WORK or
        // SHUTDOWN.
      }
      // On shutdown, break out of the for loop.
      if (SHUTDOWN == m_State) break;
      // Set state to indicate no work is queued.
      m_State = NO_WORK;
    }
 
    // do some work here
  }
}

Note: As a personal preference, I opted to not allocated boost::thread on the heap, and as a result, I do not need to manage it via boost::scoped_ptr. boost::thread has a default constructor that will refer to Not-a-Thread, and it is move-assignable.

---

Related Topics