Pete wrote:
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.
[example snipped]
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.
A thread_group is documented as destroying added thread objects when its own destructor is called (see http://boost.org/libs/thread/doc/thread.html#class-thread_group-synopsis). Your example should probably be changed to: int main(int argc, char* argv[]) { for (int j = 0; j < 2; ++i) { boost::thread_group threads; //MOVED THIS LINE for (int i = 0; i < 10; ++i) threads.create_thread(&increment_count); threads.join_all(); } }
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
This is dealt with in the version of Boost.Thread which is in the thread_dev branch of cvs, which I'm slowly moving into the main branch.
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
Looking at the code, this appears to be correct: the thread* is removed from the internal list but is never deleted. If you remove the thread from the group, you are responsible for deleting it. 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
This, as you discovered, is false.
or b) the thread_group is deleted
This is true.
- after returning from a call to join_all() the boost::thread_group has no boost::thread object members
This, as you discovered, is also false.
- 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
This is true, but only in the thread_group destructor. It would probably be a good idea for the documentation to make all of these issues clearer, as you suggest at the top of your post.
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?
This is probably a good idea.
How about thread objects "remembering" their membership in a thread_group so that the thread group can be informed when a thread terminates?