Don't use the `volatile` qualifier for multi-threaded values

[michaellenaghan]

Reading through the code I noticed the use of volatile values to communicate between threads, like this:

https://github.com/facil-io/cstl/blob/835bd33990d4d388366f44a295757ab9f798a8ed/fio-stl/102%20queue.h#L523

which is defined here:

https://github.com/facil-io/cstl/blob/835bd33990d4d388366f44a295757ab9f798a8ed/fio-stl/102%20queue.h#L79

That is, most definitely, not safe.

See, for example, this site with a rather cute name:

• https://isvolatileusefulwiththreads.com/c/

The second link is broken, but I found another copy of the article here. Here's an excerpt:

For multi-threaded programming, there are two key issues that volatile is often mistakenly thought to address:

• atomicity
• memory consistency, i.e. the order of a thread's operations as seen by another thread.

Let's deal with (1) first. Volatile does not guarantee atomic reads or writes. ...

Now consider issue (2). Sometimes programmers think of volatile as turning off optimization of volatile accesses. That's largely true in practice. But that's only the volatile accesses, not the non-volatile ones. ...

The StackOverflow link also has some good content, e.g.:

The problem with volatile in a multithreaded context is that it doesn't provide all the guarantees we need. It does have a few properties we need, but not all of them, so we can't rely on volatile alone.

However, the primitives we'd have to use for the remaining properties also provide the ones that volatile does, so it is effectively unnecessary.

and:

C programmers have often taken volatile to mean that the variable could be changed outside of the current thread of execution; as a result, they are sometimes tempted to use it in kernel code when shared data structures are being used. In other words, they have been known to treat volatile types as a sort of easy atomic variable, which they are not. The use of volatile in kernel code is almost never correct; this document describes why.

Finally, here is a short answer given by Herb Sutter, a C/C++ super-expert:

I am implementing multi producer single consumer problem. I have shared variables like m_currentRecordsetSize which tells the current size of the buffer. I am using m_currentRecordsetSize in a critical section do i need to declare it as volatile.

If you’re in C or C++, and the variable is not already being protected by a mutex or similar, then you need to declare it atomic (e.g., if it’s an int, then atomic_int in C or atomic in C++. Not volatile.

[boazsegev]

I am aware of these misunderstanding, but I believe that I used volatile correctly within the library.

I will re-read through some of the code, to see which write operations need to be replaced with attic write operations (to enforce in instruction fence / memory barrier) , especially since write operations tent to "float" to the to if nothing before them contains a memory barrier... but I think most if not all are used correctly.

The keyword volatile is not enough for multi-threaded code, but it is an important part of some concurrent code as it forces the compiler to produce mov instructions and re-read certain variables from memory (instead of using a cached copy from a CPU register). This slows code execution but it is important especially when using flags, such as in the example you gave.

Yes, volatile doesn't guaranty atomic operations or ordering (in practice, some operations are atomic anyway when dealing with single instruction operations that operate on aligned word sized variables, which is what facil.io often does).

But if you will read through the code you will notice that it uses volatile mostly for reading flags and write operations to the flags are mostly performed using atomic instructions (which include both a memory and instruction barrier that prevents instruction reordering, where there's no need for instruction ordering the code sometimes skip atomics).

This, to my understanding, is exactly the use case for volatile that we need (similar to what one would do in a signal handler).

FYI: the volatile keyword was used originally because I had to run the code on some systems that didn't support C11 atomics and I was using compiler builtins (see here, for example)... I still aim for C99 support as I may need it again.

Note:

Most of the warnings about volatile are in reference to large structures, not word-sized variables. The library doesn't use volatile with structs (except when trying to test code execution speeds, at which point volatile is there to prevent the compiler from optimizing the testing loop away).

[michaellenaghan]

I thought that your use of volatile would be considered; that's why I went a little over the top with links and quotes. :-) It's also why I called out a specific example. That same example is used in the linked articles to explain why such usage of volatile is wrong!

I specifically tried to point to some particularly authoritative — i.e., "highly reliable" — sources. I'm not aware of any authoritative source that carves out the kind of exception you're describing for volatile?

(similar to what one would do in a signal handler)

Yes, signal handlers are one of three "existing portable uses" of volatile given by Hans Boehm (yet another authoritative source). But there's a critical caveat: you have to use it with a specific type, a type that's guaranteed to be atomic on the given platform. Here's the way the libc manual explains it:

To avoid uncertainty about interrupting access to a variable, you can use a particular data type for which access is always atomic: sig_atomic_t. Reading and writing this data type is guaranteed to happen in a single instruction, so there’s no way for a handler to run “in the middle” of an access.

The type sig_atomic_t is always an integer data type, but which one it is, and how many bits it contains, may vary from machine to machine.

Btw, Boehm's post also includes this quote from David Butenhof:

the use of volatile accomplishes nothing but to prevent the compiler from making useful and desirable optimizations, providing no help whatsoever in making code 'thread safe'

Butenhof is the author of Programming with POSIX Threads.

[boazsegev]

Thank you for reaching out to me with your concerns and for opening this issue. The debate is important and I fixed some missing atomics in the FIO_QUEUE module.

However, I will close the issue for now, please free to open a new issue (or re-open this one) if you have specific points in the code you think I should fix.

Also, note:

The while (!grp->stop) use of volatile as a flag was not the mistake in the example code here. The issue in the example concerned the instruction re-ordering by the compiler (the missing instruction barrier that would be provided by atomics) and not the use of volatile...