Safe to use volatile bool to force another thread to wait? (C++)

Question

Everything I've read about volatile says it's never safe, but I still feel inclined to try it, and I haven't seen this specific scenario declared unsafe.

I have a separate thread that renders a scene, pulling data from the main simulation thread. This has no synchronization, and works fine.

The issue is that when the program exits, then renderer needs to stop pulling data from the simulation thread before the simulation thread can safely clean itself up without causing the renderer to attempt reading invalid memory.

To accomplish this, I have the renderer run infinitely in its thread:

volatile bool stillRendering;

void RenderThreadFunction()
{
    stillRendering = true;

    while(programRunning)
    {
        renderer->render();
    }

    stillRendering = false;
}

In the main program thread, when the windproc quit message is received, I do:

void OnQuit()
{
    programRunning = false;
    while(stillRendering)
    {
    }

    delete application;
}

The goal of this is to be sure the renderer stops pulling data from the application before calling delete on the application.

I first tried this without any volatile keywords, and it worked in debug mode, but in release mode it hung. I assume the compiler made some optimization that causes the program to stop checking the value of stillRendering.

Adding volatile to just stillRendering caused the application to successfully exit everytime I've tested it so far. I'm not certain why it doesn't seem to matter if "programRunning" is volatile.

Lastly, I am uncertain how the performance of the program will be impacted by using volatile for "stillRendering". It doesn't matter to me if making stillRendering volatile affects the performance of OnQuit(), but it does matter to me if it affects the performance of RenderThreadFunction()

Answer 1

It's completely unsafe, although it might work with some compilers. Basically, volatile only affects the variable it's attached to, so RendererThreadFunction, for example, could set stillRendering false before having finished renderer->render();. (This is true even if both stillRendering and programRunning were both volatile.) The probablility of a problem is very small, so testing probably won't reveal it. And finally, some versions of VC++ do give volatile the semantics of an atomic access under C++11, in which case, your code will work. (Until you compile with a different version of VC++, of course.)

Given that renderer->render() almost certainly takes a non-negligible amount of time, there's absolutely no reason for not using a conditional variable here. About the only time you'd use volatile for this sort of thing is if the shutdown mechanism were triggered by a signal (in which case, the type would be sig_atomic_t, and not bool, although in practice, it probably doesn't make any difference). In that case, there wouldn't be two threads, but just the renderer thread and a signal handler.

Answer 2

If you want your code to work on all architectures in all compilers, use C++11 atomics:

std::atomic<bool> stillRendering;

void RenderThreadFunction()
{
    stillRendering = true;

    while(programRunning)
    {
        renderer->render();
    }

    stillRendering = false;
}

Volatile is not designed for use with multi-threading -- compilers are actually allowed by the standard to reorder volatile accesses with non-volatile accesses. VC++ extends volatile's feature set to prevent reordering, but other compilers do not, and it might break on those compilers.

As others have mentioned, volatile also doesn't affect visibility, meaning architectures which aren't cache-coherent may never see the flag set. x86 isn't even immediately cache-coherent (writes would be extremely slow), so your program will invariably end up looping more than it should while the write is sent through various buffers.

C++11 atomics avoid both of these problems.

OK, so this was mainly meant to correct your current code and warn you off of misusing volatile. James' suggestion of using a condition variable (which is merely a more efficient version of what you're doing) is probably the best actual solution for you.

Answer 3

There are three issues that C++11 atomics address.

First, a thread switch can occur in the middle of reading or writing a value; for reads, another thread could update the value before the original thread reads the rest of the value; for writing, another thread could see the half-written value. This is known as "tearing".

Second, in a typical multi-processor system, each processor has its own cache, and it reads and writes values to that cache; sometimes the cache and main memory are updated to ensure that they hold the same values, but until a processor that writes a new value flushes its cache and a thread that reads the value reloads its copy from the cache the value can be different. This is referred to as "cache coherency".

Third, the compiler can move code around, and store one value before it stores another, even if the code is written in the opposite order. As long as you can't write a valid program that can see the difference, that's okay under the "as if" rule.

Loading from and storing to an atomic variable (with the default memory-ordering) prevents all three problems. Marking a variable as volatile does not.

EDIT: don't bother figuring out which architectures pose which problems. The author of the standard library has already done this for the architecture that the library implementation is intended for. Don't look for shortcuts; just use atomics. You won't lose anything.

Answer 4

Depending on the architecture being cache-coherent (e.g. x86 processors) I expect this will work just fine. You may find that either of your two threads may run for an iteration more than if you use true atomic operations, but since only one side is setting vs. reading the values, there is no requirement for true atomic operations.

However, if the processor (cores) that are executing the code require specific cache-flushing to make the "other core(s)" see the updated value, then you may be stuck for some time - and you would need proper atomic updates to ensure the other processor's cache is invalidated.

I'm assuming that renderer->render() takes a fair amount of time, so reading the stillRendering shouldn't affect the overall runtime much at all. volatile typically just means "please don't put this in a register and keep it there".

(You probably need programRunning to be a volatile too!)

Answer 5

Adding volatile to just stillRendering caused the application to successfully exit everytime I've tested it

Yes, your scenario will work.

The common mistake that occurs when using volatile variables for thread synchronization is when operations on volatile variables are presumed to be atomic. They aren't.

In your case, you are polling a single bool waiting for it to change exactly once to exactly 0. You don't seem to be expecting any operation to be atomic. On the other hand, even if you were polling a single int, C++ will not guarantee that a thread changing that int will do so atomically.

I'm not certain why it doesn't seem to matter if "programRunning" is volatile.

It does matter. Make it volatile.

Making a variable volatile will guarantee that specific cache optimizations are avoided, which is what you want.

That doesn't mean that you are guaranteed those same cache optimizations when a variable isn't volatile. You're simply letting the compiler decide. And at this particular time, the compiler happens to be making a decision that works for you.

Lastly, I am uncertain how the performance of the program will be impacted by using volatile for "stillRendering".

Your performance will likely be negatively affected by this:

while(stillRendering)
{
}

You are asking one thread (perhaps one entire CPU core) to endlessly, without rest, read a single variable.

Consider adding a sleep call in that while loop.