Subject: Re: [boost] [thread 1.48] Multiple interrupt/timed_join leads to deadlock
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2012-12-07 14:53:37
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
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()
thread* create_thread(F threadfunc);
Create a new boost::thread object as-if by new thread(threadfunc) and add it to the group.
this->size() is increased by one, the new thread is running.
A pointer to the new boost::thread object.
Member function add_thread()
void add_thread(thread* thrd);
The expression delete thrd is well-formed and will not result in undefined behaviour.
Take ownership of the boost::thread object pointed to by thrd and add it to the group.
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.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk