Le 07/12/12 20:26, Gaetano Mendola a écrit :
On 07/12/2012 19.03, Vicente J. Botet Escriba wrote:
Le 07/12/12 01:23, Gaetano Mendola a écrit :
Even the implementation can be improved, even if it's thread safe it doesn't avoid to issue a join to an already joined thread, or for the matter to interrupt a thread already joined/interrupted.
I don't know if you comment has something to be with these two tickets
https://svn.boost.org/trac/boost/ticket/7668: thread_group::join_all() should check whether its threads are joinable
Intersting, I saw the fix and as you can see it has a race condition there, indeed a thread can detach itself between the call to joinable and the join at that point even if the check was made when the join is issued the precondition doesn't hold anymore.
Basically that is the question I'm asking: how can we be sure that a thread is joinable when we call the join on it, for sure checking the "joinable" has some problems. I have solved this in my thread group implementation with a flag saying if the thread was already joined or not and this works only because our threads do not detach them self so the thread group is the only responsible for the status change.
From my point of view a thread in a thread_group is owned by the thread group. Any direct operation on a thread belonging to the thread group falls in undefined behavior. From the documentation Member function create_thread() template<typename F> thread* create_thread(F threadfunc); Effects: Create a new boost::thread object as-if by new thread(threadfunc) and add it to the group. Postcondition: this->size() is increased by one, the new thread is running. Returns: A pointer to the new boost::thread object. Member function add_thread() void add_thread(thread* thrd); Precondition: The expression delete thrd is well-formed and will not result in undefined behaviour. Effects: Take ownership of the boost::thread object pointed to by thrd and add it to the group. Postcondition: this->size() is increased by one.
https://svn.boost.org/trac/boost/ticket/7669: thread_group::join_all() should catch resource_deadlock_would_occur
That is another story about the exception safety of thread_group, and as you can see it's not exception safe. Please, let me know where there is an issue and I'll try to fix it.
I also believe the create_thread returning a boost::thread pointers is very dangerous, indeed doing so the boost::thread_group doesn't have anymore the ownership of it, it would be better if create_thread would have returned a thread id instead. I don't want to invest to much on the thread_group class, as for me this class was designed before threads were movable and a complete redesign should be done. You can of course create a Trac ticket requesting this new feature.
Due the limitation of this class I have wrote my own thread group with also the method join_any having the semantic to interrupt all threads if any of the threads terminate. I believe seeing the current status of thread_group class the best to do is or fix it for good or deprecate it. I'm for deprecating it, as I posted already sometime ago, but of course I'm for fixing the bugs I could be introduced by recent modification in other parts of the library. Anyway, before deprecating it, we need an alternative. Maybe you have already a concrete proposal and you can submit it to Boost.
Best, Vicente