std::condition_variable::wait_for

[Christopher Pisz]

What kind of syntax do I use when trying to call
std::condition_variable::wait_for and I have a bool variable that I want
to use to control whether or not the predicate argument is true or false?


I currently have

while(m_stopCV.wait_for(lock, std::chrono::seconds(60),
std::move(m_stop)) == std::cv_status::timeout)

where
m_stopCV is a std::condition_variable &
lock is a local std::unique_lock
m_stop is a bool &

Also, I need m_stop to be protected by the mutex when it is evalutated.
How do I do that while using this function?


--
I have chosen to troll filter/ignore all subthreads containing the
words: "Rick C. Hodgins", "Flibble", and "Islam"
So, I won't be able to see or respond to any such messages
---

[Christopher Pisz]

On 9/1/2015 2:05 PM, Christopher Pisz wrote:
>
> What kind of syntax do I use when trying to call
> std::condition_variable::wait_for and I have a bool variable that I want
> to use to control whether or not the predicate argument is true or false?
>
>
> I currently have
>
> while(m_stopCV.wait_for(lock, std::chrono::seconds(60),
> std::move(m_stop)) == std::cv_status::timeout)
>
> where
> m_stopCV is a std::condition_variable &
> lock is a local std::unique_lock
> m_stop is a bool &
>
> Also, I need m_stop to be protected by the mutex when it is evalutated.
> How do I do that while using this function?
>
>

I think I got it. Lamda Expression capturing this.
while(!m_stopCV.wait_for(lock, std::chrono::seconds(10), [this](){return
m_stop;}) )

[Scott Lurndal]

Christopher Pisz <nos...@notanaddress.com> writes:
>
>What kind of syntax do I use when trying to call
>std::condition_variable::wait_for and I have a bool variable that I want
>to use to control whether or not the predicate argument is true or false?
>
>
>I currently have
>
>while(m_stopCV.wait_for(lock, std::chrono::seconds(60),
>std::move(m_stop)) == std::cv_status::timeout)

The canonical way is:

pthread_mutex_lock(&lock);
while (!predicate) {
pthread_cond_wait(&cv, &lock); /* or pthread_cond_timedwait(); */
}
pthread_mutex_unlock(&lock);

std::condition_variable::wait_for() does all that
for you when the third (predicate) argument is a
function returning bool.

If you don't have a function returning bool, but rather
are accessing memory directly, then you need to worry about
volatile accesses to the predicate vis-a-vis compiler
optimizations.

[Chris Vine]

On Tue, 01 Sep 2015 14:56:39 -0500
Christopher Pisz <nos...@notanaddress.com> wrote:

> On 9/1/2015 2:05 PM, Christopher Pisz wrote:
> >
> > What kind of syntax do I use when trying to call
> > std::condition_variable::wait_for and I have a bool variable that I
> > want to use to control whether or not the predicate argument is
> > true or false?
> >
> >
> > I currently have
> >
> > while(m_stopCV.wait_for(lock, std::chrono::seconds(60),
> > std::move(m_stop)) == std::cv_status::timeout)
> >
> > where
> > m_stopCV is a std::condition_variable &
> > lock is a local std::unique_lock
> > m_stop is a bool &
> >
> > Also, I need m_stop to be protected by the mutex when it is
> > evalutated. How do I do that while using this function?
> >
> >
>
> I think I got it. Lamda Expression capturing this.
> while(!m_stopCV.wait_for(lock, std::chrono::seconds(10),
> [this](){return m_stop;}) )

This is not right. The return value of the version of
std::condition_variable::wait_for() which takes a predicate indicates
whether the predicate is true or not when the timeout occurs (it
returns false if there was a timeout without the predicate becoming
true, otherwise true). You don't want to loop on it again: it already
contains its own while loop to guard against spurious wake-ups (see
§30.5.1/32). Also, will this code compile without the lambda body being
'return this->m_stop' - I can't remember if there is some implicit
application of the this pointer to m_stop in the lambda body but I
thought not?

In summary, your while loop effectively overrides the timeout. If you
don't want a timeout, use plain std::condition_variable::wait().

On your mutex point, you always call
std::condition_variable::wait{for|until}() with the mutex locked. If
the condition variable waits it releases the mutex by itself. On
awakening it automatically reacquires the mutex. So m_stop is always
tested under the mutex. One other consequence of this is that no more
than one thread can proceed at a time if you do a broadcast on the
condition variable - each thread after the first will block until it can
acquire the mutex after the previously acquiring thread has released it.

Chris

[Chris Vine]

On Tue, 01 Sep 2015 20:02:26 GMT
sc...@slp53.sl.home (Scott Lurndal) wrote:
> The canonical way is:
>
> pthread_mutex_lock(&lock);
> while (!predicate) {
> pthread_cond_wait(&cv, &lock); /* or
> pthread_cond_timedwait(); */ }
> pthread_mutex_unlock(&lock);
>
> std::condition_variable::wait_for() does all that
> for you when the third (predicate) argument is a
> function returning bool.
>
> If you don't have a function returning bool, but rather
> are accessing memory directly, then you need to worry about
> volatile accesses to the predicate vis-a-vis compiler
> optimizations.

No you don't, unless your "accessing memory directly" involves things
like mmap'ed memory which another process may be mutating.

So far as concerns threads, volatile is never sufficient because it
doesn't avoid a data race (it doesn't synchronise) and only serves to
suppress acceptable optimizations for multi-threaded code. Nor is it
necessary - atomic variables and mutexes act as compiler reordering
barriers as well as carrying out synchronization. Volatile is a waste
of time with threads.

And in relation to condition variables, since you enter a condition
variable wait with the mutex locked, the condition variable test will
automatically be synchronized by the mutex.

Chris

[Christopher Pisz]

That's not the behavior I am seeing. It outputs "tick..." every 10
seconds, as expected.

I want to loop, because the loop is where the task for the thread is
done. Unless you are implying that the work goes in the lamda..are you?

> On your mutex point, you always call
> std::condition_variable::wait{for|until}() with the mutex locked.

Isn't that what I am doing?

std::unique_lock<std::mutex> lock(m_stopMutex); <--locks?

while(!m_stopCV.wait_for(lock, std::chrono::seconds(10), [this](){return
m_stop;}) )

{

> If
> the condition variable waits it releases the mutex by itself. On
> awakening it automatically reacquires the mutex. So m_stop is always
> tested under the mutex.

Good, I was hoping so. I couldn't find it explicitly stated in the doc.

> One other consequence of this is that no more
> than one thread can proceed at a time if you do a broadcast on the
> condition variable - each thread after the first will block until it can
> acquire the mutex after the previously acquiring thread has released it.

Only when the broadcast is done? That would be ok, because all they are
going to do is go and exit. As long as the body of the while loop can be
executed in more than one thread at a time.

[Christopher Pisz]

Just realized this post doesn't have the full context, but only the one
line of code, since I was just asking about the wait_for here. Please
see the later separate post on threading for full listing.

[Chris Vine]

On Tue, 01 Sep 2015 18:00:31 -0500

Christopher Pisz <nos...@notanaddress.com> wrote:
> On 9/1/2015 5:34 PM, Chris Vine wrote:
> > On Tue, 01 Sep 2015 14:56:39 -0500
> > Christopher Pisz <nos...@notanaddress.com> wrote:

[snip]

> >> I think I got it. Lamda Expression capturing this.
> >> while(!m_stopCV.wait_for(lock, std::chrono::seconds(10),
> >> [this](){return m_stop;}) )
> >
> > This is not right. The return value of the version of
> > std::condition_variable::wait_for() which takes a predicate
> > indicates whether the predicate is true or not when the timeout
> > occurs (it returns false if there was a timeout without the
> > predicate becoming true, otherwise true). You don't want to loop
> > on it again: it already contains its own while loop to guard
> > against spurious wake-ups (see §30.5.1/32). Also, will this code
> > compile without the lambda body being 'return this->m_stop' - I
> > can't remember if there is some implicit application of the this
> > pointer to m_stop in the lambda body but I thought not?
> >
> > In summary, your while loop effectively overrides the timeout. If
> > you don't want a timeout, use plain std::condition_variable::wait().
>
> That's not the behavior I am seeing. It outputs "tick..." every 10
> seconds, as expected.

Then you are outputting the ticks within the while loop. Fine if it is
for debugging purposes, but otherwise at variance with the purpose of
a condition variable. (Your code snippet above contained no code which
printed anything - it just spun on a timeout without any purpose.
More generally, looping on a timeout is also usually pointless,
particularly if the condition is one to end the thread rather than to
do some work. If the condition was one about the availability of work
it would make a bit more sense.)

> I want to loop, because the loop is where the task for the thread is
> done. Unless you are implying that the work goes in the lamda..are
> you?

If you are doing work in the while loop above then it must surely be
wrong to have gratuitous waits on a condition variable related to
something completely different, namely whether a stop flag is set?

> > On your mutex point, you always call
> > std::condition_variable::wait{for|until}() with the mutex locked.
>
> Isn't that what I am doing?

Yes. I wasn't arguing that your code is wrong on this point, but you
asked a question and I answered it. I think you may be conflating this
series of posts with your one containing a request for a code review,
which I haven't read.

[snip]

> > One other consequence of this is that no more
> > than one thread can proceed at a time if you do a broadcast on the
> > condition variable - each thread after the first will block until
> > it can acquire the mutex after the previously acquiring thread has
> > released it.
>
> Only when the broadcast is done? That would be ok, because all they
> are going to do is go and exit. As long as the body of the while loop
> can be executed in more than one thread at a time.

You have lost me on while loops (the while loop above which is dependent
on a timeout occurring and does nothing is only executed one thread at a
time because it executes under the mutex, but I rather think you are
talking about something else). I was just explaining how condition
variables work, as I sensed from your question about mutexes that you
weren't clear. Possibly at some point in whatever while loop you have
you need to release the mutex, and acquire it again before looping.

Chris

[Christopher Pisz]

What's the alternative?

>>> On your mutex point, you always call
>>> std::condition_variable::wait{for|until}() with the mutex locked.
>>
>> Isn't that what I am doing?
>
> Yes. I wasn't arguing that your code is wrong on this point, but you
> asked a question and I answered it. I think you may be conflating this
> series of posts with your one containing a request for a code review,
> which I haven't read.
>
> [snip]
>
>>> One other consequence of this is that no more
>>> than one thread can proceed at a time if you do a broadcast on the
>>> condition variable - each thread after the first will block until
>>> it can acquire the mutex after the previously acquiring thread has
>>> released it.
>>
>> Only when the broadcast is done? That would be ok, because all they
>> are going to do is go and exit. As long as the body of the while loop
>> can be executed in more than one thread at a time.
>
> You have lost me on while loops (the while loop above which is dependent
> on a timeout occurring and does nothing is only executed one thread at a
> time because it executes under the mutex, but I rather think you are
> talking about something else). I was just explaining how condition
> variables work, as I sensed from your question about mutexes that you
> weren't clear. Possibly at some point in whatever while loop you have
> you need to release the mutex, and acquire it again before looping.
>
> Chris
>

Ok, let's move this discussion to the post with the full listing and
discuss the full listing. I just wanted to get past the syntax error for
the one line in this OP.

Once over there,
Can you please write an example there with the following requirements in
standard C++?

1) Create a main thread
2) Have the main thread create a number of child threads with tasks
encapsulated in an object
3) Have the main thread wait on all children to exit before it, itself
exits.
4) Have all children continue to do their tasks every 10 seconds, in a
loop forever, until told to stop from the main thread
5) Have the main thread handle ctrl-c and tell all children to stop in
response.

I have no idea how to edit my code given your responses. I am just
further confused.

[Chris Vine]

Your "Have all children continue to do their tasks every 10 seconds"
seems completely mad. Why on earth have worker threads gratuitously
doing nothing for 10 seconds on every iteration between tasks,
irrespective of whether there is work available. You must surely mean
something else?

If that really is what you want then maybe your design might be OK
(subject to my point about mutex locking), but as I say it is so outside
anything I could contemplate as a reasonable design aim that I couldn't
say more. It is sufficiently bizarre that you ought to have explained
this in your request for a code review.

If this is some kind of automated machinery controller that does
something (maybe polls machinery) every 10 seconds I doubt you would
need threads at all: the fact that each thread must sleep for 10
seconds for this to work shows that a single thread would be more than
sufficient and threads are the wrong solution. Threads are supposed to
do things, not not do things.

Chris

[Chris Vine]

On Wed, 2 Sep 2015 01:37:01 +0100
Chris Vine <chris@cvine--nospam--.freeserve.co.uk> wrote:
[snip]

> If this is some kind of automated machinery controller that does
> something (maybe polls machinery) every 10 seconds I doubt you would
> need threads at all: the fact that each thread must sleep for 10
> seconds for this to work shows that a single thread would be more than
> sufficient and threads are the wrong solution. Threads are supposed
> to do things, not not do things.

I would suggest a standard single-threaded program event loop with a
timer for each piece of machinery. That bit of the code would require
about 5 lines of code, including handling program termination.

Chris

[Christopher J. Pisz]

I want separate threads so that each will run completely independent of
the other.

Often when polling there is some situation where one has a failure and
must retry continuously. On a single thread, that will hold up all the
others.

Also, processing of the data can take more than the wait time seconds,
especially since it is coming over unreliable, third party http, so
again, I want each to run independently.

I've got a single threaded solution already. I aim to take 90% of it and
plop it on a thread of its own, rather than running multiple clients.

I don't see how that is uncommon at all. I've seen the same pattern at
least thirty times in my career, just not with C++11.

[Christopher J. Pisz]

Oh, I should mention, I am not polling machinery, or bunches of them.
Something like 10-20 data sources per client.

[Christopher J. Pisz]

On 9/1/2015 7:55 PM, Chris Vine wrote:

I do wonder though reading through the condition_variable docs, it
mentions spinning while waiting. If it actually consumes cycles while
doing nothing, it would be better not to, but I don't see any
alternatives in the standard. I'd have to resort to Windows threads.

There is no event or other semaphore available, is there?

[Ian Collins]

Where does it say that? A thread waiting on a CV blocks until the CV is
signaled, or something else wakes it (a spurious wake-up). This could
consume cycles, but it would be a poor implementation if it did.

--
Ian Collins

[Chris Vine]

On Tue, 01 Sep 2015 21:15:24 -0500
"Christopher J. Pisz" <cp...@austin.rr.com> wrote:
[snip]

> I want separate threads so that each will run completely independent
> of the other.
>
> Often when polling there is some situation where one has a failure
> and must retry continuously. On a single thread, that will hold up
> all the others.
>
> Also, processing of the data can take more than the wait time
> seconds, especially since it is coming over unreliable, third party
> http, so again, I want each to run independently.
>
> I've got a single threaded solution already. I aim to take 90% of it
> and plop it on a thread of its own, rather than running multiple
> clients.
>
> I don't see how that is uncommon at all. I've seen the same pattern
> at least thirty times in my career, just not with C++11.

OK, if that's your problem area and you are happy with your approach of
a thread per device (and there are not so many devices that you are
going to run into resource problems), so be it. However, although I
haven't looked at the posting which contained your code other than very
cursorily, it looks like you need to deal with the locking, assuming
all threads share the m_stopCV and m_stopMutex objects, because at
present only one thread will run its task at a time. But that is
easily resolved.

Your polling will not run at exactly 10 second intervals. It will be
10 seconds plus the time to run the task. If you want the time to run
the task discarded, you would need to use
condition_variable::wait_until() with a separate time_point object for
the thread, which you reset by a further 10 seconds at each firing.

Chris

[Chris Vine]

On Wed, 02 Sep 2015 01:21:36 -0500

"Christopher J. Pisz" <cp...@austin.rr.com> wrote:

Condition waits don't spin, unless possibly speculatively for a few
nanoseconds, so you don't have a problem on that score: they don't use
up clock cycles.

Chris

[Christopher Pisz]

They appear to be running in parallel according to the log. They
certainly aren't going one every ten seconds. I'll try with a longer task.

If that is a problem, how do we easily solve it?

> Your polling will not run at exactly 10 second intervals. It will be
> 10 seconds plus the time to run the task. If you want the time to run
> the task discarded, you would need to use
> condition_variable::wait_until() with a separate time_point object for
> the thread, which you reset by a further 10 seconds at each firing.
>
> Chris
>

Yea, I realized that. I'll do some kind of time arithmetic from start of
task to end.

Log:

2015-09-01 18:05:48.144017|ThreadedOngoingTask #9 tick...
2015-09-01 18:05:48.144017|ThreadedOngoingTask #7 tick...
2015-09-01 18:05:48.144017|ThreadedOngoingTask #6 tick...
2015-09-01 18:05:48.144017|ThreadedOngoingTask #5 tick...
2015-09-01 18:05:48.144017|ThreadedOngoingTask #3 tick...
2015-09-01 18:05:48.144017|ThreadedOngoingTask #2 tick...
2015-09-01 18:05:48.144017|ThreadedOngoingTask #8 tick...
2015-09-01 18:05:48.144017|ThreadedOngoingTask #1 tick...
2015-09-01 18:05:48.144017|ThreadedOngoingTask #4 tick...
2015-09-01 18:05:48.144017|ThreadedOngoingTask #0 tick...
2015-09-01 18:05:58.149118|ThreadedOngoingTask #0 tick...
2015-09-01 18:05:58.149118|ThreadedOngoingTask #4 tick...
2015-09-01 18:05:58.149118|ThreadedOngoingTask #3 tick...
2015-09-01 18:05:58.149118|ThreadedOngoingTask #2 tick...
2015-09-01 18:05:58.149118|ThreadedOngoingTask #1 tick...
2015-09-01 18:05:58.149118|ThreadedOngoingTask #7 tick...
2015-09-01 18:05:58.149118|ThreadedOngoingTask #6 tick...
2015-09-01 18:05:58.149118|ThreadedOngoingTask #8 tick...
2015-09-01 18:05:58.149118|ThreadedOngoingTask #5 tick...
2015-09-01 18:05:58.149118|ThreadedOngoingTask #9 tick...

[Chris Vine]

On Wed, 02 Sep 2015 10:15:37 -0500
Christopher Pisz <nos...@notanaddress.com> wrote:
[snip]

> They appear to be running in parallel according to the log. They
> certainly aren't going one every ten seconds. I'll try with a longer
> task.
>
> If that is a problem, how do we easily solve it?

[snip]

> Log:
>
> 2015-09-01 18:05:48.144017|ThreadedOngoingTask #9 tick...
> 2015-09-01 18:05:48.144017|ThreadedOngoingTask #7 tick...
> 2015-09-01 18:05:48.144017|ThreadedOngoingTask #6 tick...
> 2015-09-01 18:05:48.144017|ThreadedOngoingTask #5 tick...
> 2015-09-01 18:05:48.144017|ThreadedOngoingTask #3 tick...
> 2015-09-01 18:05:48.144017|ThreadedOngoingTask #2 tick...
> 2015-09-01 18:05:48.144017|ThreadedOngoingTask #8 tick...
> 2015-09-01 18:05:48.144017|ThreadedOngoingTask #1 tick...
> 2015-09-01 18:05:48.144017|ThreadedOngoingTask #4 tick...
> 2015-09-01 18:05:48.144017|ThreadedOngoingTask #0 tick...
> 2015-09-01 18:05:58.149118|ThreadedOngoingTask #0 tick...
> 2015-09-01 18:05:58.149118|ThreadedOngoingTask #4 tick...
> 2015-09-01 18:05:58.149118|ThreadedOngoingTask #3 tick...
> 2015-09-01 18:05:58.149118|ThreadedOngoingTask #2 tick...
> 2015-09-01 18:05:58.149118|ThreadedOngoingTask #1 tick...
> 2015-09-01 18:05:58.149118|ThreadedOngoingTask #7 tick...
> 2015-09-01 18:05:58.149118|ThreadedOngoingTask #6 tick...
> 2015-09-01 18:05:58.149118|ThreadedOngoingTask #8 tick...
> 2015-09-01 18:05:58.149118|ThreadedOngoingTask #5 tick...
> 2015-09-01 18:05:58.149118|ThreadedOngoingTask #9 tick...

This tells you nothing. To "prove" that the tasks are running
concurrently you would have to have two interleaved log lines, which
your logger probably prevents anyway (I haven't looked at it).

Look at the while loop in the ThreadedOngoingTask::ThreadProcedure()
method, look at the state of m_stopMutex there (put in some debugging
code which reports on the value returned by
std::unique_lock::owns_lock() in the loop at the point where you call
the logger, which I assume substitutes in your test case for "doing
something" in the worker thread, and then it should be obvious what the
problem is and what to do, namely call std::unique_lock::unlock() and
std::unique_lock::lock() at the appropriate places. This assumes that
m_stopMutex and m_stopCV are shared by all worker threads.

Chris

[Christopher Pisz]

Ah, I see. Thanks for catching that.
So, I something like this, is OK?


// Do not allow exceptions to escape the thread
try
{
// Loop forever until signalled to exit

std::unique_lock<std::mutex> lock(m_stopMutex);

while(!m_stopCV.wait_for(lock, std::chrono::seconds(10),
[this](){return m_stop;}) )

{
// Unlock such that other worker threads can run thier tasks
lock.unlock();

// Do the work
std::ostringstream msg;
msg << "ThreadedOngoingTask #" << m_id << " tick...Owns
Lock: " << lock.owns_lock();
Shared::Logger::getInstance()->logExecutionEvent(msg.str(),
__FILE__, __LINE__, Shared::DateTime(),
Shared::Logger::LoggingLevel::LOG_LEVEL_DEBUG);

// Lock for the next while condition check
lock.lock();
}
}
catch(...)
{
std::ostringstream msg;
msg << "An unhandled exception occured in thread of
ThreadedOngoingTask #" << m_id << ". Stopping thread.";
Shared::Logger::getInstance()->logExecutionEvent(msg.str(),
__FILE__, __LINE__, Shared::DateTime(),
Shared::Logger::LoggingLevel::LOG_LEVEL_DEBUG);
}

[Chris Vine]

On Wed, 02 Sep 2015 12:07:18 -0500

That looks fine.

Chris