[Interest] Heavily Commented Example: Simple Single Frontend with Two BackendsHi,

[K. Frank]

Hi Daniel!

On Wed, Oct 24, 2012 at 2:08 AM, Daniel Bowen
<qtmailinglist1 at bowensite.com> wrote:
>> >> Let me state for the record that I do not use volatiles for thread
>> >> synchronization.  But the issue at hand is not whether a volatile
>> >> can be used for full-featured thread synchronization, but whether
>> >> it can be used by one thread to signal a second looping thread to
>> >> quit.
>> >
>> > It can. In that restricted scenario, even a non-atomic write would
>> > be sufficient.
>
> (snip)
>
>> > CPU A writes to the volatile signalling variable, but it writes to its
>> > CPU-specific cache.  The thread that's looping runs on CPU B, and
>> > repeatedly reads from the volatile signalling variable, but always
>> > reads it from its own CPU-specific cache.  So it never gets the signal
>> > to quit, and potentially loops forever.
>>
>> Well, that's not exactly how processors work. CPU A will eventually get
>> to write the data from its cache back to main RAM. And CPU B will
> eventually
>> get to notice that and discard its cache. So the code running on CPU B
> will
>> eventually get to see the new value.
>>
>> The question is only how long that might take.
>>
>> Also note that you should not implement a busy-wait loop like that, like
>> a spinlock. At least on x86, if one CPU is constantly reading from the
> same
>> address, it will prevent the others from writing to it. Since you're
>> busy- looping, you're spending power.
>
> I'm also quite interested in this topic. There are a handful of places where
> I've used a similar pattern where one thread is doing a loop, and reading a
> stop flag to stop a while loop, and some other thread can write to that stop
> flag.  That stop flag is a member variable.  The loops where I've used it
> with a stop variable are not constantly pounding the CPU unnecessarily.  For
> example, let's say you have a thread that auto saves any changes on a
> regular interval, or one that processes "work items" if there are any.  And
> a QWaitCondition is used to wake up to either do work, or stop.  But if
> there isn't anything to do, it's doing a smart wait.

I would comment that since you're already using a condition variable,
you're using a mutex, and you can easily avoid the issues discussed
here about using a volatile stop flag for (this kind of) thread
synchronization.

> For example:
>
> while(!m_stop)
> {
>         m_lock.lock();
>         AutoSave();
>         if(!m_stop)
>         {
>                 m_waitCondition.wait(&m_lock, m_autoSaveInterval);
>         }
>         m_lock.unlock();
> }

To avoid any issues with using a volatile m_stop, you can take
advantage of the mutex you already have and are already
synchronizing on.  Something like:

   while (true) {
      m_lock.lock();
      AutoSave();
      if (m_stop)  // break out of loop, release mutex, and stop
      else m_waitCondition.wait(&m_lock, m_autoSaveInterval);
      m_lock.unlock();
   }

Or slightly better (because condition_variable.wait (m_lock) has the
release and reacquisition of m_lock built in):

   m_lock.lock();
   while (true) {
      AutoSave();
      if (m_stop)  break;
      else m_waitCondition.wait(&m_lock, m_autoSaveInterval);
   }
   m_lock.unlock();
   // do whatever to stop

>
> With some other method called from another thread like
>
> bool A::stopAndWait(unsigned long timeoutMs)
> {
>         m_stop = true;
>         m_waitCondition.wakeAll();
>         return this->wait(timeoutMs);
> }

Following up on the scheme of using your existing mutex to protect
m_stop, and noting that you need to own the condition variable's
mutex in order to call condition-variable.wakeAll() (or for
std::condition_variable, condition_variable.notify_all()):

   bool A::stopAndWait (unsigned long timeoutMs) {
      m_lock.lock();
      m_stop = true;
      m_waitCondition.wakeAll();
      m_lock.unlock();
      return this->wait(timeoutMs);
   }


>
> So, in that case, I've typically declared
> volatile bool m_stop;

With the above modification that protects m_stop with the mutex you
already have, m_lock, you no longer need the volatile (and the issues
it raises).

> With the intention of using volatile for "don't optimize the read away".
>
> After reading some various links like the Herb Sutter article referenced
> earlier, a co-worker believes that volatile in this case is unnecessary.
> i.e., just use
> bool m_stop;

I think your co-workers are right, because your essential use of m_stop
(inside your "while(!m_stop)" loop)

   m_lock.lock();
   AutoSave();
   if(!m_stop)
   {
      m_waitCondition.wait(&m_lock, m_autoSaveInterval);
   }
   m_lock.unlock();

is protected by m_lock, forcing a proper read of m_stop, so that
when the loop re-executes its while condition (while(!m_stop)),
the value of m_stop will be up to date, even though m_stop is
unprotected in the while-loop condition.

If the rest of the code were a little perverse, I think hypothetically
that your code could fail.  Let's say m_stop were initialized to
true.  Then thread A sets m_stop to false and starts thread B,
which then calls the "while(!m_stop)" loop.  I believe that because
that first read of m_stop is unprotected, you could have a race
condition, and m_stop could evaluate to its stale value of true,
causing the loop to stop immediately.

>
> So, I'd like to understand the possibilities for the best cross-platform
> code.

Personally, I would go with the explicit mutex-protected use of m_stop
I outlined above.  You're 95% of the way there anyway.

> Going "one time too many" is OK.
> - Is reading and writing to a bool m_stop (no volatile) without a mutex
> locked OK?

In general, clearly not.

(But in your case, as I mentioned above, your essential read of m_stop
is explicitly protected by the mutex, and the essential write to m_stop
is presumably protected by the mutex because you go on to call
"m_waitCondition.wakeAll();" implying that the mutex was held.  So in
your specific case, you're probably okay.)

> Or could the read of the member variable m_stop realistically be
> optimized away?

In general, yes.

> - Is reading and writing to a volatile bool m_stop without a mutex locked
> OK?

Thiago has convinced me that for this purpose volatile is not actually
good enough.

> - If no locking is OK, is volatile better for m_stop, or does it not matter
> (and just causes a little bit slower execution for the read/write of
> m_stop)?

I don't think that no locking is okay.  But if your code were to work without
volatile (and without locking) then the reads and writes must be occurring,
so adding volatile, which prohibits the compiler from optimizing the reads
and writes away, should have no effect (because the reads and writes are
anyway there), so the code should not run any more slowly with volatile.

> - Is not using volatile on bool m_stop OK, but both reading and writing to
> m_stop should be done with a mutex locked?

Yes.  This is the way I would do it, and, as I understand it, the way it
should be done.

> - If m_stop is only read or written with a mutex locked, could the value
> ever be stale that is read (causing "one time too many")?

No.  (Barring a bug in your code or the compiler or the os.)

> - What if m_stop was QAtomicInt instead of bool?

I don't know the details of Qt's atomic semantics, so I don't know
whether your scheme should work with QAtomicInt.  In general,
atomics were designed to address these kinds of issues, but I don't
think that plain-vanilla atomic variables are quite strong enough to do
what we want here.  Unless you make use of cross-thread memory
fences, I think of atomics as preventing you from seeing inconsistent
data, but not necessarily preventing you from seeing stale data.

> Thanks!
> -Daniel

Anyway, that's my story, and I'm sticking to it.

Happy Multi-Threaded Hacking!


K. Frank