Subject: Re: [boost] [Thread][Release] Boost.Thread and 1.45 release
From: vicente.botet (vicente.botet_at_[hidden])
Date: 2010-10-29 08:30:26
----- Original Message -----
From: "Anthony Williams" <anthony.ajw_at_[hidden]>
To: <boost_at_[hidden]>
Sent: Friday, October 29, 2010 8:52 AM
Subject: Re: [boost] [Thread][Release] Boost.Thread and 1.45 release
>
> "vicente.botet" <vicente.botet_at_[hidden]> writes:
>
>> From: "Anthony Williams" <anthony.ajw_at_[hidden]>
>
>>> I've fixed all the outstanding "showstopper" bugs in boost.thread on
>>> trunk (including the thread interruption issue, #2330),
>
>> I have take a look at the modification associated to this ticket #2330
>> thread::interrupt() can be lost if condition_variable::wait() in
>> progress and I don't reach to understand why do you need to have an
>> internal mutex on condition_variable. The associated patch don't
>> modifies this point. The resolution don't states nothing more than the
>> ticket has been solved but no how. Please could you explain this
>> addition? Is this related to another ticket?
>
> The patch attached to the ticket is incomplete. There is no requirement
> that the same mutex is used with the same condition variable instance
> for every wait. Therefore, if a thread waits with one mutex, wakes,
> destroys the mutex and then waits with another then there is a race
> condition with another thread that tries to interrupt it --- the
> interrupting thread might try and lock a just-destroyed mutex.
Oh, I see. So to support interruptions, the condition variable needs to be associated to a single mutex which must be locked before any condition variable wait.
I don't understand however why do you need to lock the mutex each time you signal the condition
inline void condition_variable::notify_one()
{
*** boost::pthread::pthread_mutex_scoped_lock internal_lock(&internal_mutex);
BOOST_VERIFY(!pthread_cond_signal(&cond));
}
inline void condition_variable::notify_all()
{
*** boost::pthread::pthread_mutex_scoped_lock internal_lock(&internal_mutex);
BOOST_VERIFY(!pthread_cond_broadcast(&cond));
}
void thread::interrupt()
{
detail::thread_data_ptr const local_thread_info=(get_thread_info)();
if(local_thread_info)
{
lock_guard<mutex> lk(local_thread_info->data_mutex);
local_thread_info->interrupt_requested=true;
if(local_thread_info->current_cond)
{
*** boost::pthread::pthread_mutex_scoped_lock internal_lock(local_thread_info->cond_mutex);
BOOST_VERIFY(!pthread_cond_broadcast(local_thread_info->current_cond));
}
}
}
Please, could you clarify what are you protecting with the lock (***)?
I was wondering why do we need to pass a unique_lock to the wait operations if it is not needed at all?
> The internal mutex is necessary to eliminate this race condition. This
> mutex could be global, but that would introduce a point of contention
> between all cond vars.
>> This will make condition_variable and condition_variable_any almost
>> equivalent. What will be the adavantage to use condition_variable
>> respect to condition_variable_any?
>
> The only difference now is that condition_variable_any will accept any
> type of mutex in the wait functions. There is no difference between
> them in terms of performance.
If I remember the restriction on condition_variable was to be able to be as efficient as if we used the underlying platform. If we don't have any advantage for condition_variable, why to have it? As not all applications use the interruption mechanism, I will preserv a condition_variable class that is not an interruption point and is as efficient as possible.
Best,
Vicente