Re: [boost] [thread] Interaction between interruption points and condition_variable

Subject: Re: [boost] [thread] Interaction between interruption points and condition_variable
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2017-06-19 16:30:41

Le 19/06/2017 à 08:12, Vicente J. Botet Escriba via Boost a écrit :
> Le 16/06/2017 à 00:28, David Stone via Boost a écrit :
>> Assume
>>
>> 1) A thread is waiting on a condition_variable
>> 2) That condition_variable has been notified that its condition is 
>> now true
>> 3) The thread has been interrupted
>>
>> Given this, the interruption is processed rather than the
>> condition_variable unblocking normally regardless of the order in 
>> which the
>> notify and the interruption occur. That means that this program would
>> eventually fail even though the notify always comes before the 
>> interrupt.
>>
>>
>>
>>
>> #include <boost/thread/condition_variable.hpp>
>> #include <boost/thread/lock_types.hpp>
>> #include <boost/thread/mutex.hpp>
>> #include <boost/thread/thread.hpp>
>>
>> #include <cassert>
>>
>> struct flag_t {
>>      using lock_type = boost::unique_lock<boost::mutex>;
>>
>>      void notify() {
>>          auto lock = lock_type(m_mutex);
>>          m_flag = true;
>>          m_cv.notify_one();
>>      }
>>
>>      void wait() {
>>          auto lock = lock_type(m_mutex);
>>          m_cv.wait(lock, [=]{ return m_flag; });
>>      }
>>
>> private:
>>      bool m_flag = false;
>>      boost::mutex m_mutex;
>>      boost::condition_variable m_cv;
>> };
>>
>> struct test_t {
>>      unsigned value = 0;
>>      ~test_t() {
>>          assert(value != 0);
>>          assert(value != 1);
>>          assert(value == 2);
>>      }
>> };
>>
>> int main() {
>>      while (true) {
>>          flag_t flag;
>>          test_t test;
>>
>>          auto thread = boost::thread([&]{
>>              test.value = 1;
>>              boost::this_thread::interruption_requested();
>>              flag.wait();
>>              test.value = 2;
>>          });
>>
>>          flag.notify();
>>          thread.interrupt();
>>          thread.join();
>>      }
>> }
>>
>>
>>
>>
>> If we move the `flag.notify()` line before the thread creation, this
>> program never terminates. This is because
>> `boost::condition_variable::wait(lock_type &, function)` is defined 
>> as only
>> blocking when the function returns false. The only way this program can
>> terminate is if `thread` begins execution and executes `flag.wait()` 
>> before
>> `flag.notify()` is called, then `thread.interrupt()` runs before 
>> `thread`
>> is unblocked.
>>
>> It seems more intuitive to me that if a thread is blocked on a
>> condition_variable and it is interrupted that it would unblock normally
>> rather than throwing boost::thread_interrupted. Under that behavior, 
>> users
>> who want the current behavior can always check after they leave a
>> condition_variable::wait using
>> `boost::this_thread::interruption_requested()` and throw rather than
>> processing data. Under the current behavior, I do not believe there 
>> is any
>> way to process the data in a function that does not have access to the
>> thread object without losing the fact that an interruption was 
>> requested.
>> This would only be possible if we had a function like
>> `boost::this_thread::interrupt()`, but that does not exist.
>>
>> I do not know what code (if any) depends on the current behavior of
>> interruptions always taking precedence over notifications, but it 
>> appears
>> that the current behavior is undocumented. Is there a justification 
>> for the
>> current behavior or is it just the way it happened to be implemented? Is
>> this something we could change, or at least allow users to implement the
>> behavior I outlined?
>>
>>
> Hi,
>
>
> I believe I understand what is happening. Wait is defined as 
> equivalent to
>
>
> while(!pred())
> {
>     wait(lock);
> } but it should be defined as equivalent to 
> boost::this_thread::interruption_requested(); while(!pred())
> {
>     wait(lock);
> }
>
>
Sorry fro the format. I believe it should be

if (pred()) boost::this_thread::interruption_point();

while(!pred())

{
     wait(lock);
}

Vicente
>
> that is that every wait like function should have the same 
> requirements of the wait function.
>
> The documentations says the following for wait
>
>
> http://www.boost.org/doc/libs/1_64_0/doc/html/thread/synchronization.html#thread.synchronization.condvar_ref.condition_variable.wait 
>
>
> Throws:
>
>    |boost::thread_resource_error| if an error occurs.
>    |boost::thread_interrupted| if the wait was interrupted by a call to
>    |interrupt()|
> <http://www.boost.org/doc/libs/1_64_0/doc/html/thread/thread_management.html#thread.thread_management.thread.interrupt>
>    on the |boost::thread|
> <http://www.boost.org/doc/libs/1_64_0/doc/html/thread/thread_management.html#thread.thread_management.thread>
>    object associated with the current thread of execution.
>
>
> I believe this ia a bug, and adding the proposed line shouldn't break 
> any working  code
>
> Would this work for you?
>
>
> Best,
>
> Vicente
>
>
> _______________________________________________
> Unsubscribe & other changes: 
> http://lists.boost.org/mailman/listinfo.cgi/boost