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 06:12:05

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);
}

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