Re: [boost] [Thread][Release] Boost.Thread and 1.45 release

Subject: Re: [boost] [Thread][Release] Boost.Thread and 1.45 release
From: vicente.botet (vicente.botet_at_[hidden])
Date: 2010-10-29 12:05:03

----- Original Message ----- 
From: "Anthony Williams" <anthony.ajw_at_[hidden]>
To: <boost_at_[hidden]>
Sent: Friday, October 29, 2010 3:53 PM
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]>
>>> "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
> 
> If another thread locks the external mutex and then calls notify_one()
> then either (a) the waiting thread must be waiting (and will therefore
> wake), or (b) it has not yet locked the mutex (and so hasn't started
> waiting). In order to preserve the atomicity of the unlock-and-wait
> from the point of view of the notifying code, the internal mutex must be
> locked when calling pthread_cond_signal, since the external mutex is
> unlocked before the call to pthread_cond_wait.

OK. I understand now the need.
 
>> I was wondering why do we need to pass a unique_lock to the wait
>> operations if it is not needed at all?
> 
> It *is* needed --- the external mutex is not unlocked until *after* the
> internal mutex has been locked.
>  
>> 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.
> 
> It's a trade off.
> 
> boost::condition_variable::wait has always been advertised as an
> interruption point, and in most cases it has worked. Unfortunately,
> there was a race condition on POSIX that sometimes caused it to not see
> the interrupt until after being woken. Fixing the race condition
> requires adding an internal mutex, which will have a performance impact.
> 
> Removing condition_variable::wait() as an interruption point will break
> people's code where they have been relying on it. This will be the worst
> kind of breakage --- silent breakage, since the external interface would
> not have changed in any way. Windows programmers would also lose the
> facility, even though it has always worked on Windows.

Yes, I agree that breaking user code is not an option. But I think yet that an efficient condition variable is needed even if it can not be an interruption point. I see two paths:

A- Use of conditional compilation to enable the efficient implementation.
B- Add a new class stating clearly that it doesn't works with interruptions, and signal in the documentation that uses of condition_variable that needs to be interrupted should move to condition_variable_any and that condition_variable wil not support them 3 or 4 version later. Then you could restablish the efficient version on condition_variable.

I recognize that this seems complex, but I don't see a better way to respect the current users, those that expect condition_variable be un interruption poit and those that expects condition_variable is as efficient as possible when interrupts are not considered.

Best,
Vicente