Work at SourceForge, help us to make it a better place! We have an immediate need for a Support Technician in our San Francisco or Denver office.

Close

#83 Allow real join() for threads of ThreadPool

open
nobody
Other (7)
5
2012-09-14
2008-08-21
Anonymous
No

We use the Poco's HTTP server. We need to start server inside our application on demand, later we stop it by some event. We also required to continue application execution after stopping the server only when all threads used by the server are actually finished (in other words the Thread::join() are required for all pool's threads).

But unfortunately the Poco's ThreadPool doesn't provide the method to join() ("real" join()) to all its threads after stopping pool working.

The ThreadPool::joinAll() actually just wait for completion of all tasks executed by the pool's threads but not the completion of the threads.

The ThreadPool::stopAll() actually lead to the finishing of all threads, but it not join() to these threads ! After stopAll() returns the pool's threads can be still executed (going to stopping).

We propose to add ability to join to all pool's threads when pool is no longer used and going to be destroyed. It can be bool parameter of the stopAll(bool joinToAll) which indicate whether stopAll() need to join to all threads (new behavior) or just turn the threads to going stopping and exit (old behavior).

Sorry for bad English !

Best regards, Alexey Kryshen

A.Kryshen@gmail.com

Discussion

  • Jiang
    Jiang
    2008-08-26

    Logged In: YES
    user_id=2096293
    Originator: NO

    Hey Alexey Kryshen

    I don't quite get your point here.

    It is stated explicitly (in ThreadPool.h) that

    /// Creating and starting a threads can impose a significant runtime
    /// overhead to an application. A thread pool helps to improve
    /// the performance of an application by reducing the number
    /// of threads that have to be created (and destroyed again).
    /// Threads in a thread pool are re-used once they become
    /// available again.
    

    So ThreadPool::joinAll will just waits for all thread complete and set the wait event for next activation. That's the main point for Poco::ThreadPool in my mind.

    If you want to destroy the thread objects, you need ThreadPool::stopAll, which will stop all threads and also delete all of the thread objects. Is this really you want?

    HTH

    Jiang

     
  • Logged In: NO

    Hi, Jiang !

    Please note that when it is said that ThreadPool::stopAll() stop all threads it is actually mean that threads are just turned to going to stopping but thread's routines are not actually terminate its execution right at this moment! It is how threads are work. Also note that destroying the Poco's Thread object is not guarantee that thread routine are finished too.

    In a case of standalone thread to be sure the thread routine are actually finished there are method Thread::join(). But for ThreadPool there are no way to do same for all its threads. That is what I am talking about and why we propose to add this ability to ThreadPool::stopAll(). It would be very helpful if ThreadPool::stopAll() will actually do Thread::join() for all pool's threads before destroying thread objects.

    Best regards, Alexey Kryshen
    A.Kryshen@gmail.com

     
  • Logged In: NO

    Hey Alexey Kryshen

    Now somehow I know what the problem. What I see is you need the automatic thread cancellation.

    Well, if I read the code correctly, POCO::thread related classes do not provide the so-called automatic thread cancellation support. It is client's responsibility to synchronize the main thread and the [pooled] worker threads. That is, threads shall end/exit by themselves before main thread need to join them. Or, the client code must do the necessary synchronization for thread cancellation, which means, the worker threads shall usually check necessary quit flag(event, notification...) and the main thread shall set the quit flags whenever the cancellation command is issued.

    Thread cancellation, especially asynchronous thread cancellation, is hard to do correctly/safely. Although we have cancellation specification in POSIX, but not all implementations implement it. Since one of the main points of POCO is cross-platform, I do not think this issue can be solved easily in the recent future.

    So, my suggestion is, rewrite your thread code to deal with this issue by yourself. Or, you can have a look at Poco::Task and Poco::TaskManager, which provide more state control for the underlying tasks, and the embedded notification queue and cancelAll() are maybe useful for your above purpose.

    Generally speaking, the Poco::ThreadPool is quite suitable for HTTP server implementation, but for your special use-case (need [frequent] shutdown/restart, and maybe block connection used), you need sorts of "special" treatments.

    Just my 2 cents, hope there are better solutions for you application.

    HTH

    Jiang

     
  • Logged In: NO

    Hi, Jiang !

    Actually all that we need is Thread::join() be called for pool's worker threads after ThreadPool::stopAll() notify them need to be stopped. It can be done either in the stopAll() internally or through separate method. It will give the full control under ThreadPool.

    Lets see some code (see my comments for more details):

    I) How ThreadPool::stopAll() work now

    void ThreadPool::stopAll()
    {
    FastMutex::ScopedLock lock(_mutex);

    for (ThreadVec::iterator it = _threads.begin(); it != _threads.end(); ++it)
    {
        (*it)->release(); /// It will notify worker thread that it must be finished, but not wait for it !!!
    }
    _threads.clear(); /// Just destroy the thread objects, not wait its completions too
    

    }

    II) Our proposal for stopAll()

    void ThreadPool::stopAll(bool iJoin)
    {
    FastMutex::ScopedLock lock(_mutex);

    for (ThreadVec::iterator it = _threads.begin(); it != _threads.end(); ++it)
    {
        (*it)->release(); /// It will notify worker thread that it must be finished, but not wait for it !!!
                if(iJoin)
                {
                  (*it)->joinToThread(); /// New method need to be added to the PooledThread, it is not the current PooledThread::join() !!!
                                         /// After this method returns it is guaranteed that worker thread are finished (what we need)
                }
    }
    _threads.clear(); /// Just destroy the thread objects
    

    }

    And the joinToThread() must be defined as :

    void PooledThread::joinToTread()
    {
    _thread.join();
    }

    Now you must see the difference in how it is now and how we propose it be.

    Best regards, Alexey Kryshen
    A.Kryshen@gmail.com

     
  • Logged In: NO

    Hey Alexey

    I will try to explain what I see, hope it makes things clearer.

    1. Fact1:

    Poco::ThreadPool is designed to solve the problem of costly thread construction/destruction. That means, ThreadPool object does not really join any pooled threads during its life cycle. Here please note "join" has the same meaning as pthread_join. Think ThreadPool's join as an emulated one please.

    1. Fact2:

    In your application, you need " all threads used by the server are actually finished (in other words the
    Thread::join() are required for all pool's threads). "

    So you see the problem, Poco::ThreadPool, designed as a general pooled-thread manager, does not really join threads. This is a fundamental design policy. Therefore it does not fit your special needs very well as I said in my previous post.

    And for your above proposed code, since

    1. It will break the above design policy for general thread pool.

    2. Worse, if even just one of your worker threads fails to finish its job timely (socket block listening, for example), the main thread will hang.

    , I don't think it is a feasible patch for Poco library.

    Well, for your above use-case, please consider:

    1. Do not use Poco::ThreadPool. Also the name is quite attractive for your usage, as I explained just now, it does not really fit your application. You can build your own thread manager without much troubles by referring Poco::Task and Poco::TaskManager.

    2. If you really want to use the Poco::ThreadPool interface, then please build your own ThreadPool class. More specifically, the patch you proposed is not enough, you need to take care of the thread create/start/join issues too. Anyway, maybe it should not be called thread pool any more.

    3. Although this is not a good idea, you can destroy the thread pool object and reconstruct it when restarted.

    Again, just my 2 cents.

    HTH

    Jiang

     
  • Hi, Jiang

    1) Please note that our proposal is just for case of terminating/destroing the ThreadPool. In such a case "Fact1" doesn't matter.
    2) I also can't accept your second argument ("Fact2"). Why general pooled-thread manager can't join() (pthread_join()) to its threads during termination ? I do not see any reasons ... The "possible" hangs can't be an argument because it is possible in current implementation as well if you has logical errors in your program (just call ThreadPool::joinAll() for pool in which at least one Runable is endless because any kind of hangs and you will get hang of thread called joinAll(); if you will call stopAll() instead in such situation you will end up with "ghost" hanged thread after ThreadPool is destroyed - not so good also ...).

    Best regards, Alexey Kryshen

    A.Kryshen@gmail.com

     
  • Hi, Jiang !

    Sorry for late answer.

    The ThreadPool::joinAll waits for completion of "jobs" executed in real OS's threads. Please see the POCO's source and you will see that.

    In my case main interest point in wait for completion of real OS's threads created by the ThreadPool which is completelly impossible with current POCO API.

    Best regards, Alexey Kryshen