[Boost-users] boost::thread_group design flaw

From: Pete (petehug_at_[hidden])
Date: 2004-03-16 17:46:08

I'm pretty convinced that there is a serious design flaw in
boost::thread_group or at the very minimum, the documentation should better
address the issue.

Let's take a look at the thread_group.cpp example provided with
boost::thread:

#include <boost/thread/thread.hpp>
#include <iostream>

int count = 0;
boost::mutex mutex;

void increment_count()
{
    boost::mutex::scoped_lock lock(mutex);
    std::cout << "count = " << ++count << std::endl;
}

int main(int argc, char* argv[])
{
    boost::thread_group threads;
    for (int i = 0; i < 10; ++i)
        threads.create_thread(&increment_count);
    threads.join_all();
}

This example works fine until I make the following small change to main()
which breaks it:

int main(int argc, char* argv[])
{
    boost::thread_group threads;
    for (int j = 0; j < 2; ++i)
    {
      for (int i = 0; i < 10; ++i)
          threads.create_thread(&increment_count);
      threads.join_all();
    }
}

The above code will break the second time threads.join_all() is called. The
problem is that all boost::thread objects created in the first iteration
survive the join_all() call. Worse still, the 2nd join_all() call sends
another join() message to each thread it created but boost::thread::join()
does not check the m_joinable flag and simply tries to free the system
resources it previously freed again causing a system fault.

To correct the problem above, I can change the code as follows:

int main(int argc, char* argv[])
{
    boost::thread*          pt[10];
    boost::thread_group threads;
    for (int j = 0; j < 2; ++i)
    {
      for (int i = 0; i < 10; ++i)
          pt[i] = threads.create_thread(&increment_count);
      threads.join_all();
      for (int i = 0; i < 10; ++i)
          threads.remove_thread(pt[i]);
    }
}

While this code no longer causes a GPF, I do not believe that the
boost::thread objects created by boost::thread_group::create_thread()
actually ever get destroyed. I have set a break point in
boost::thread::~thread() but this breakpoint is never reached. If I
physically delete each pt[i] after removing it from the thread_group I'm
getting an assertion error telling me that the pointer I'm trying to delete
is not a valid debug-heap pointer, but this error will most likely disappear
if I stop using the debug heap (I haven't tried yet).

As a boost::thread_group user, I made the following assumptions:

- a boost::thread created through boost::thread_group::create_thread will be
removed and cleaned up properly when a) the thread terminates or b) the
thread_group is deleted

- after returning from a call to join_all() the boost::thread_group has no
boost::thread object members

- I need not specifically keep track of boost::thread objects created by
boost::thread_group and destruction of these objects is encapsulated within
boost::thread_group

I now find that none of these assumptions is quite correct. Perhaps I good
immediate remedy would be to implement a boost::thread_group::clear() method
which would throw an error if any of the threads where still in a running
state and else would clear its threads collection and destroy the objects?
How about thread objects "remembering" their membership in a thread_group so
that the thread group can be informed when a thread terminates?