Is volatile needed?

Question

if I have a byte queue, where it is expected to have one thread producer, another consumer:

class ByteQueue{
    byte[] buf;
    /*volatile?*/ int readIdx;
    /*volatile?*/ int writeIdx;
    Runnable writeListener;
    Runnable readListener;
    // ...
    void write( byte[] b ){
        int wr = writeIdx;
        int rd = readIdx;
        // check consistency and free space using wr+rd
        // copy to buf, starting at wr, eventually wrap around
        // update writeIdx afterwards
        writeIdx = ( wr + b.length ) % buf.length;
        // callback to notify consumer for data available
        writeListener.run();
    }
    void read( byte[] b ){
        int wr = writeIdx;
        int rd = readIdx;
        // check consistency and available data using wr+rd
        // copy buf to b, starting at rd, eventually wrap around
        // update readIdx afterwards
        readIdx = ( rd + b.length ) % buf.length;
        // callback to notify producer for free space available
        readListener.run();
    }
    int available() { return (writeIdx - readIdx) % buf.length; }
    int free() { return buf.length - available() -1; }
    // ...
}

This type of queue should not need synchronization.
readIdx is only modified by reader thread,
writeIdx only by the writer thread.

readIdx == writeIdx means, there is no content.
And the queue can only take up to buf.length-1 bytes of data.

Are the volatiles needed or can they be omitted because only one thread is the modifier of one integer state?

thx Frank

You need even stronger synchronization, `writeIdx` updates must always happen after `buf` updates. It's going to be tricky without `synchronized`. — Piotr Praszmo, Jun 30 '15 at 19:05
Agree with Banthar. Play it safe, don't take shortcuts. Write proper synchronization. Don't sacrifice correctness to gain a nanosecond. — sstan, Jun 30 '15 at 19:07
You're right about the modification, but don't forget that reads are also happening. — David Ehrmann, Jun 30 '15 at 19:08
Can `available()` be called when any other thread is actively using the `read()` or `write()` methods? If so, then what is it supposed to mean? (Hint: The best you can do is return the number of bytes that _were_ available at some instant in time, but that won't necessarily be the same as the number of bytes that _are_ available by the time the caller decides to do something about it.) — Solomon Slow, Jun 30 '15 at 19:12

Nathan Hughes · Accepted Answer · 2015-06-30T19:11:29.313

5

If another thread has to read it, it needs to be volatile. The volatile keyword indicates to the JVM that the value can't be cached or have its updates reordered, otherwise updates to its value may not be visible to other threads.

The visibility concern extends to the buf array too. Since buf needs to change in step with the indexes it would seem like the write and read methods need to be synchronized. Synchronization makes the changes visible and makes sure that concurrent calls don't result in the indexes and buf contents becoming inconsistent.

edited Jun 30 '15 at 19:11

answered Jun 30 '15 at 19:04

Nathan Hughes

94,330
19
181
276

You can use `volatile` to handle the contents of the buffer too. The trick is to remember that reading and writing a `volatile` field establishes a happens-before relationship between threads, so if thread A writes bytes in the buffer before updating a volatile field, and then thread B examines the buffer _after_ examining the same volatile field, then thread B should see the bytes that thread A wrote into the buffer. But you are right, `synchronized` is the more obvious way... – Solomon Slow Jun 30 '15 at 19:16
@james: agreed, piggybacking could be made to work for the visibility angle, if the OP wants to go that way. – Nathan Hughes Jun 30 '15 at 19:19
Thanks that helped. So i will make them volatile. synchronized is not needed, as writeIdx is updated last, the reading cannot happen before and the other way around. – fbenoit Jun 30 '15 at 19:51
@keinfarbton: yes, this is what James describes in his comment where you can rely on the happens-before relationship created by the volatile variables to make sure you see the updates to the buffer. – Nathan Hughes Jun 30 '15 at 20:03

score 2 · Answer 2 · answered Jun 30 '15 at 19:07

You should declare them volatile. Let's look, for example, at readIdx. If it it not volatile, writer thread optimization can assume it is never changed and make wrong optimizations based on that assumption.

However, I don't see that you access readIdx anywhere in writer thread (or writeIdx in reader thread) apart for assignment to some local variable rd (or wr). I just assume there is some code missing or otherwise your question doesn't really make sense.

score 1 · Answer 3 · answered Jun 30 '15 at 19:11

Nathan is correct, it's not that the two threads would write over each others variables, but that the variables themselves would risk to never be visible for the other thread (or rather CPU-core).

There is an interesting queue that actually uses non-volatile variables to let the CPU better schedule the work, the LMAX disruptor.

Is volatile needed?

3 Answers3