Deadlock with thread group's join_all and other modifier methods.

[JDKunk@…]

In boost::thread_group, the join_all function locks mutex m. So in other functions which use lock_guard, the lock_guard locks mutex m creating deadlock with multiple modifiers to the thread_group. The solution includes changing join_all so the mutex is only locked upon obtaining the next element of the thread_group's thread. Since a std::list is used, using an iterator while making it possible for another thread to modify the list is unacceptable. The solutions is to pop the std::list like a queue. So the front thread is dequeued while the mutex is locked, then the mutex is unlocked when the thread is joined. Thus it is possible for multiple threads to modify the thread_group while join_all has occurred.

This code represents the current problem with the join_all function and other modifiers.

void join_all()
{
  boost::shared_lock<shared_mutex> guard(m);
...

void interrupt_all()
{// This accessor has no problem with join_all
 boost::shared_lock<shared_mutex> guard(m);
...
void remove_thread(thread* thrd)
{// This modifier has no problem with join_all
  boost::lock_guard<shared_mutex> guard(m);
...

The below code is my minimal test case for this problem.

#include <boost/thread.hpp>

boost::mutex lock;
boost::thread * t[128];
boost::thread_group tg;

void run(int i) {
  lock.lock();
  // remove_thread locks the thread_group's mutex,
  // and join_all locks the thread_group's mutex
  // causing deadlock.
  tg.remove_thread(t[i]); // ***** Deadlock when tg.m is locked.
}

int main() {
  lock.lock();
  for( int i = 0; i < 128; ++i ) {
    t[i] = new boost::thread(run,i);
    tg.add_thread(t[i]);
  }
  lock.unlock();
  tg.join_all(); // ***** Deadlock when tg.m is locked.
}

Finally here is my solution to the given problem.

void join_all()
{
    boost::thread * t;
    while( 1 ) {
        {
            boost::lock_guard<shared_mutex> guard(m);
            if( threads.empty() )
              break;
            t = threads.front();
            threads.pop_front();
        }
        t->join();
    }
}

Thank you, Jeff Kunkel

[Jeff Kunkel <JDKunk@…>]

Woops, I made a mistake on my test case. I never unlocked the mutex in run, and I never returned from main.

#include <boost/thread.hpp>

boost::mutex lock;
boost::thread * t[128];
boost::thread_group tg;

void run(int i) {
  lock.lock();
  // remove_thread locks the thread_group's mutex,
  // and join_all locks the thread_group's mutex
  // causing deadlock.
  tg.remove_thread(t[i]); // ***** Deadlock when tg.m is locked.
  lock.unlock();
}

int main() {
  lock.lock();
  for( int i = 0; i < 128; ++i ) {
    t[i] = new boost::thread(run,i);
    tg.add_thread(t[i]);
  }
  lock.unlock();
  tg.join_all(); // ***** Deadlock when tg.m is locked.
  return 0;
}

[Jeff Kunkel <JDKunk@…>]

I revised the solution to the problem. The join all method will copy the current thread list. The join all method will then join all the current threads in the list. All modifications to the list will be ignored until join_all exits and is called again. Other solutions are welcome, but this is a classic reader-writer problem.

[viboes]

Your examples is an example of deadlock, but I'm not sure if we can say that there is a deadlock in Boost.Thread.

Your solution seems to not be one, as the thread calling join-all will think that all the thread in the group are joined and this could be not the case as other thread could be added to it.

I think that the best the library can do is to move one thread group to another.

Maybe the documentation should stay that access to the thread group from any thread in the group will result in a deadlock.

Other solutions?

BTW, I don't think this is a Showstopper as you can remove the thread from the group after joining all the threads.

[viboes]

Moved to support request until resolution clarified.

[viboes]

Closed as the deadlock is not in the library. Please reopen if you don't agree with resolution.