views:

319

answers:

1

I am using boost::thread_group to create(using thread_group::create_thread()) and dispatch threads. In order to limit the max thread numbers, at the end of each thread, I remove the thread from the thread_group and delete the thread itself(so that I could decide whether new threads need to be created). However it hangs somewhere between the creation and deletion of the last thread (say the 999th one of 999 in total).

My questions are:

  • is it OK to delete the thread from within itself like what I do? if not, what the best way to achieve this
  • why does my code hangs?

Below are the related code:

//1- code to create and dispatch thread

 { 
        //mutex for map<thread_id, thread*> operations 
        boost::mutex::scoped_lock lk(m_mutex_for_ptr); 

        // create a thread for this->f(duplicate_hashes) 
        boost::thread* p = m_thread_group.create_thread(boost::bind( 
            &detectiveT<equal_predicate>::f, 
            this, 
            duplicate_hashes 
            )); 

        // save the <thread_id,thread pointer> map for later lookup & deletion 
        m_thread_ptrs.insert(make_pair(p->get_id(), p)); 

        // log to console for debug 
        cout << "thread created: " 
            << p->get_id() << ", " 
            << m_thread_group.size() << ", " m_thread_ptrs.size() << 
"\n";     
    }

//2- code of the thread execution

void f(list<map_iterator_type>& l) 
{ 
    Do_something(l);    
    boost::this_thread::at_thread_exit(boost::bind( 
        &detectiveT<equal_predicate>::remove_this_thread, 
        this 
        ));                     
}

//3- code to delete the thread itself

void remove_this_thread() 
{ 

    { 
        //mutex for map<thread_id, thread*> operations 
        boost::mutex::scoped_lock lk(m_mutex_for_ptr);                   
        boost::thread::id this_id(boost::this_thread::get_id()); 

        map<boost::thread::id, boost::thread*>::iterator itr; 

        itr = (m_thread_ptrs.find(this_id)); 

        if(m_thread_ptrs.end() != itr) 
        { 
            // remove it from the control of thread_group 
            m_thread_group.remove_thread(itr->second); 
            // delete it 
            delete itr->second; 

            // remove from the map 
            m_thread_ptrs.erase(this_id); 

            // log to console for debug 
            cout << "thread erased: " 
                << this_id << ", " 
                << m_thread_group.size() << ", " 
                << m_thread_ptrs.size() << "\n";             
        } 
    }               
}

Thanks.

+3  A: 

Why don't you try to recycle the threads, since creation/destruction is expensive?

Code a thread pool class and send tasks to it. The pool will either queue the tasks if it has no more available threads, create threads if current_threads < max_threads or just use what thread is available.

Suggested implementation:

Find out what your ideal thread count is. This is usually equal to the number of processors. Depending on how complicated you want this to be, you could create all the threads in the pool at once or add threads if current-thread-count < ideal-thread-count and all the existing threads are busy executing tasks.

Assuming that you are creating all your threads at once, you need to pass a worker function to each of the threads to execute. This worker function will wait for tasks to become available and then execute them. Because the function either executes a task or waits for it, it won't return and the thread won't be destroyed.

The thread pool can keep track of a task queue and manage a wait condition that indicates when there are tasks available in the queue. Each thread worker function waits on the wait condition and when there's a task available it wakes up and tries to do the task. You will have to do some synchronization; the easiest way would be to try and find an available thread pool implementation, like the one in Windows (Vista+ I think) or the one in QtConcurrent which would allow you to just pass the task, call run and let the OS/library worry about everything.

Later edit:

Check out http://threadpool.sourceforge.net/

rpg
Can you please elaborate how to recycle a thread? My experience is that the execution of a thread is passed with the ctor, and there's no way to change it and re-use this very thread object again.
t.g.
Thanks for enlightening me, I've found Boost.Task and will try how much I can leverage with it.
t.g.
Although you did not answer my question directly, you suggested a better way to have invalid my problems. So I accepted your reply as the answer. Thanks.
t.g.
You're welcome :)
rpg