Is 'volatile' needed in this multi-threaded C++ code?

Answer 1

You should not depend on volatile to guarantee thread safety, this is because even though the compiler will guarantee that the the variable is always read from memory (and not a register cache), in multi-processor environments a memory barrier will also be required.

Rather use the correct lock around the shared memory. Locks like a Critical Section are often extremely lightweight and in a case of no contention will probably be all implemented userside. They will also contain the necessary memory barriers.

Volatile should only be used for memory mapped IO where multiple reads may return different values. Similarly for memory mapped writes.

Answer 2

Wikipedia says it pretty well.

In C, and consequently C++, the volatile keyword was intended to allow access to memory mapped devices allow uses of variables between setjmp allow uses of sig_atomic_t variables in signal handlers

Operations on volatile variables are not atomic nor establish a proper happens-before relationship for threading. This is according to the relevant standards (C, C++, POSIX, WIN32), and this is the matter of fact for the vast majority of current implementations. The volatile keyword is basically worthless as a portable threading construct.

Answer 3

volatile is neither necessary nor sufficient for multithreading in C++. It disables optimizations that are perfectly acceptable, but fails to enforce things like atomicity that are needed.

Edit: instead of using a critical section, I'd probably use InterlockedIncrement, which gives an atomic write with less overhead.

What I'd normally do, however, is hook up a thread-safe queue (or deque) as the input to the thread. When you have something for the thread to do, you just put a packet of data describing the job into the queue, and the thread does it when it can. When you want the thread to shut-down normally, you put a "shutdown" packet into the queue. If you need an immediate abort, you use the deque instead, and put the "abort" command on the front of the deque. In theory, this has the shortcoming that it doesn't abort the thread until it finishes its current task. About all that means is that you want to keep each task to roughly the same size/latency range as the frequency with which you currently check the flag.

This general design avoids a whole host of IPC problems.

Answer 4

While the other answers are correct, I also suggest you take a look at the "Microsoft specific" section of the MSDN documentation for volatile

Answer 5

I think it'll work fine (atomic or not) because you're only using it to cancel a background operation.

The main thing that volatile is going to do is prevent the variable being cached or retrieved from a register. You can be sure that it's coming from main memory.

So if I were you, yes I would define the variable as volatile.

I think it's best to assume that volatile is needed to ensure that the variable when written and then read by the other threads that they actually get the correct value for that variable as soon as possible and to ensure that IF is not optimised away (although I doubt it would be).

Answer 6

Seems that the same use case is descibed here: volatile - Multithreaded Programmer's Best Friend by Alexandrescu. It states that exactly in this case (to create flag) volatile can be perfectly used.

So, yes exactly in this case code should be correct. volative will prevent both - reading from cache and prevent compiler from optimizing out if statement.

Answer 7

Ok, so you've been beaten up enough about volatile and thread safety!, but...

An example for your specific code (though a stretch and within your control) is the fact that you are looking at your variable serveral times in the one 'transaction':

if ( ( threadParamAbort_ != NULL ) && *threadParamAbort_ )

If for whatever reason threadParamAbort_ is deleted after the left hand side and before the right hand side then you will dereference a deleted pointer. Again, this is unlikely given you have control but is an example of what volatile and atomicity can't do for you.

Answer 8

With regard to my answer to yesterday's question, no, volatile is unnecessary. In fact, multithreading here is irrelevant.

    while ( readLine( &line ) ) { // threadParamAbort_ is not local:
        if ( ( threadParamAbort_ != NULL ) && *threadParamAbort_ ) {

1. prevent the reading of *threadParameterAbort_ to use the cache and instead get the value from memory, and
2. prevent the compiler from removing the if clause in the worker thread due to optimization.

The function readLine is external library code, or else it calls external library code. Therefore, the compiler cannot assume there are any nonlocal variables it does not modify. Once a pointer to an object (or its superobject) has been formed, it might be passed and stored anywhere. The compiler can't track what pointers end up in global variables and which don't.

So, the compiler assumes that readLine has its own private static bool * to threadParamAbort_, and modifies the value. Therefore it's necessary to reload from memory.