[Thread][Release] Boost.Thread and 1.45 release
Hi, I've fixed all the outstanding "showstopper" bugs in boost.thread on trunk (including the thread interruption issue, #2330), apart from #3847 (https://svn.boost.org/trac/boost/ticket/3847) - "Unable to statically link thread lib on ia64" I don't know how to fix this, and this platform doesn't appear to be actively tested. Is it OK to merge the other fixes to the release branch? Anthony -- Author of C++ Concurrency in Action http://www.stdthread.co.uk/book/ just::thread C++0x thread library http://www.stdthread.co.uk Just Software Solutions Ltd http://www.justsoftwaresolutions.co.uk 15 Carrallack Mews, St Just, Cornwall, TR19 7UL, UK. Company No. 5478976
On 10/28/2010 7:36 AM, Anthony Williams wrote:
Hi,
I've fixed all the outstanding "showstopper" bugs in boost.thread on trunk (including the thread interruption issue, #2330), apart from #3847 (https://svn.boost.org/trac/boost/ticket/3847) - "Unable to statically link thread lib on ia64"
I don't know how to fix this, and this platform doesn't appear to be actively tested.
Then it's not a showstopper. You might try contacting the person to submitted the bug. If ia64 is important to them, maybe they'd be willing to donate test resources.
Is it OK to merge the other fixes to the release branch?
Have the tests cycled on trunk? -- Eric Niebler BoostPro Computing http://www.boostpro.com
Eric Niebler <eric@boostpro.com> writes:
On 10/28/2010 7:36 AM, Anthony Williams wrote:
Hi,
I've fixed all the outstanding "showstopper" bugs in boost.thread on trunk (including the thread interruption issue, #2330), apart from #3847 (https://svn.boost.org/trac/boost/ticket/3847) - "Unable to statically link thread lib on ia64"
I don't know how to fix this, and this platform doesn't appear to be actively tested.
Then it's not a showstopper. You might try contacting the person to submitted the bug. If ia64 is important to them, maybe they'd be willing to donate test resources.
Sounds like a good idea.
Is it OK to merge the other fixes to the release branch?
Have the tests cycled on trunk?
Yes. Anthony -- Author of C++ Concurrency in Action http://www.stdthread.co.uk/book/ just::thread C++0x thread library http://www.stdthread.co.uk Just Software Solutions Ltd http://www.justsoftwaresolutions.co.uk 15 Carrallack Mews, St Just, Cornwall, TR19 7UL, UK. Company No. 5478976
On 10/28/2010 11:41 PM, Anthony Williams wrote:
Eric Niebler <eric@boostpro.com> writes:
On 10/28/2010 7:36 AM, Anthony Williams wrote:
I've fixed all the outstanding "showstopper" bugs in boost.thread on trunk [...] Is it OK to merge the other fixes to the release branch?
Have the tests cycled on trunk?
Yes.
Please go ahead. -- Eric Niebler BoostPro Computing http://www.boostpro.com
----- Original Message ----- From: "Anthony Williams" <anthony.ajw@gmail.com> To: <boost@lists.boost.org> Sent: Thursday, October 28, 2010 4:36 PM Subject: [boost] [Thread][Release] Boost.Thread and 1.45 release
Hi,
I've fixed all the outstanding "showstopper" bugs in boost.thread on trunk (including the thread interruption issue, #2330), apart from #3847 (https://svn.boost.org/trac/boost/ticket/3847) - "Unable to statically link thread lib on ia64"
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? 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? Best, Vicente
"vicente.botet" <vicente.botet@wanadoo.fr> writes:
From: "Anthony Williams" <anthony.ajw@gmail.com>
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. 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. Anthony -- Author of C++ Concurrency in Action http://www.stdthread.co.uk/book/ just::thread C++0x thread library http://www.stdthread.co.uk Just Software Solutions Ltd http://www.justsoftwaresolutions.co.uk 15 Carrallack Mews, St Just, Cornwall, TR19 7UL, UK. Company No. 5478976
----- Original Message ----- From: "Anthony Williams" <anthony.ajw@gmail.com> To: <boost@lists.boost.org> Sent: Friday, October 29, 2010 8:52 AM Subject: Re: [boost] [Thread][Release] Boost.Thread and 1.45 release
"vicente.botet" <vicente.botet@wanadoo.fr> writes:
From: "Anthony Williams" <anthony.ajw@gmail.com>
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
"vicente.botet" <vicente.botet@wanadoo.fr> writes:
From: "Anthony Williams" <anthony.ajw@gmail.com>
"vicente.botet" <vicente.botet@wanadoo.fr> writes:
From: "Anthony Williams" <anthony.ajw@gmail.com>
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.
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. Anthony -- Author of C++ Concurrency in Action http://www.stdthread.co.uk/book/ just::thread C++0x thread library http://www.stdthread.co.uk Just Software Solutions Ltd http://www.justsoftwaresolutions.co.uk 15 Carrallack Mews, St Just, Cornwall, TR19 7UL, UK. Company No. 5478976
----- Original Message ----- From: "Anthony Williams" <anthony.ajw@gmail.com> To: <boost@lists.boost.org> Sent: Friday, October 29, 2010 3:53 PM Subject: Re: [boost] [Thread][Release] Boost.Thread and 1.45 release
"vicente.botet" <vicente.botet@wanadoo.fr> writes:
From: "Anthony Williams" <anthony.ajw@gmail.com>
"vicente.botet" <vicente.botet@wanadoo.fr> writes:
From: "Anthony Williams" <anthony.ajw@gmail.com>
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
participants (3)
-
Anthony Williams -
Eric Niebler -
vicente.botet