Search code examples
javamultithreadingvolatilememory-barriers

Volatile arrays and memory barriers and visibility in Java


I am having difficulties understanding memory barriers and cache coherence in Java, and how these concepts relate to arrays.

I have the following scenario, where one thread modifies an array (both the reference to it and one of its internal values) and another thread reads from it.

int[] integers;
volatile boolean memoryBarrier;

public void resizeAndAddLast(int value) {
    integers = Arrays.copyOf(integers, integers.size + 1);
    integers[integers.size - 1] = value;
    memoryBarrier = true;
}

public int read(int index) {
    boolean memoryBarrier = this.memoryBarrier;
    return integers[index];
}

My question is, does this do what I think it does, i.e. does "publishing" to memoryBarrier and subsequently reading the variable force a cache-coherence action and make sure that the reader thread will indeed get both the latest array reference and the correct underlying value at the specified index?

My understanding is that the array reference does not have to be declared volatile, it should be enough to force a cache-coherence action using any volatile field. Is this reasoning correct?

EDIT: there is precisely one writer thread and many reader threads.


Solution

  • Nope, your code is thread-unsafe. A variation which would make it safe is as follows:

    void raiseFlag() {
       if (memoryBarrier == true)
         throw new IllegalStateException("Flag already raised");
       memoryBarrier = true;
    }
    
    public int read(int index) {
      if (memoryBarrier == false)
         throw IllegalStateException("Flag not raised yet");
      return integers[index];
    }
    

    You only get to raise the flag once and you don't get to publish more than one integers array. This would be quite useless for your use case, though.

    Now, as to the why... You do not guarantee that between the first and second line of read() there wasn't an intervening write to integers which was observed by the second line. The lack of a memory barrier does not prevent another thread from observing an action. It makes the result unspecified.

    There is a simple idiom that would make your code thread-safe (specialized for the assumption that a single thread calls resizeAndAddLast, otherwise more code is necessary and an AtomicReference):

    volatile int[] integers;
    
    public void resizeAndAddLast(int value) {
        int[] copy = Arrays.copyOf(integers, integers.length + 1);
        copy[copy.length - 1] = value;
        integers = copy;
    }
    
    public int read(int index) {
        return integers[index];
    }
    

    In this code you never touch an array once it got published, therefore whatever you dereference from read will be observed as intended, with the index updated.