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

[David Stone via Boost]

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?

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Vicente J. Botet Escriba via Boost]

Post by David Stone via Boost
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; });
}
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


_______________________________________________
Unsubscribe & other changes: http://lists.boost.or

[Vicente J. Botet Escriba via Boost]

Post by Vicente J. Botet Escriba via Boost

Post by David Stone via Boost
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; });
}
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

Post by Vicente J. Botet Escriba via Boost
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
|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
_______________________________________________
http://lists.boost.org/mailman/listinfo.cgi/boost

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailma

[David Stone via Boost]

Actually, I am happy with the behavior that a call to wait(lock, pred)
never throws a boost::thread_interrupted exception for the case where the
predicate is true prior the call. My problem is that currently, if `thread`
is waiting on `cv`, and another thread calls

cv.notify_all();
thread.interrupt();

it is unspecified whether the call to wait returns normally or throws
boost::thread_interrupted (it depends on whether the thread that was
blocked is woken up between the two calls). I would like for it to be
specified that the call returns normally.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Rob Stewart via Boost]

Post by David Stone via Boost
Actually, I am happy with the behavior that a call to wait(lock, pred)
never throws a boost::thread_interrupted exception for the case where the
predicate is true prior the call. My problem is that currently, if `thread`
is waiting on `cv`, and another thread calls
cv.notify_all();
thread.interrupt();
it is unspecified whether the call to wait returns normally or throws
boost::thread_interrupted (it depends on whether the thread that was
blocked is woken up between the two calls). I would like for it to be
specified that the call returns normally.

That means cv.wait() is only an interruption point under certain circumstances.


--
Rob

(Sent from my portable computation device.)

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[David Stone via Boost]

Post by Rob Stewart via Boost
That means cv.wait() is only an interruption point under certain circumstances.

cv.wait is already only an interruption point only if the predicate is
false to begin with. My preferred behavior would, in some ways, maintain
that relationship. If you make the predicate true and signal, then
interrupt, and the user has not yet begun to wait, they will encounter a
true predicate and never be interrupted. If they have begun to wait (under
the current behavior), it is undefined whether they will be interrupted.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost