Necessity of volatile array write while in synchronized block

Question

Question concerning the JMM and the semantics concerning volatile fields that are written to in a synchronized block, but read un-synchronized.

In an initial version of the below code, I was not synchronizing access since it was unnecessary for earlier requirements (and abusing a self assignment this.cache = this.cache ensured volatile write semantics). Certain requirements have changed, necessitating synchronization to ensure that duplicate updates are not sent out. The question I have is does the synchronization block preclude requiring the self assignment of the volatile field?

  // Cache of byte[] data by row and column.
  private volatile byte[][][] cache;

  public byte[] getData(int row, int col)
  {
    return cache[row][col];
  }

  public void updateData(int row, int col, byte[] data)
  {
    synchronized(cache)
    {
      if (!Arrays.equals(data,cache[row][col]))
      {
        cache[row][col] = data;

        // Volatile write.
        // The below line is intentional to ensure a volatile write is
        // made to the array, since access via getData is unsynchronized.
        this.cache = this.cache;

        // Notification code removed
        // (mentioning it since it is the reason for synchronizing).
      }
    }
  }

Without synchronization, I believe that the self assignment volatile write is technically necessary (although the IDE flags it as having no effect). With the synchronized block, I think it is still necessary (since the read is unsynchronized), but I just want to confirm since it looks ridiculous in the code if it is not actually required. I am not sure if there are any guarantees that I am unaware of between the end of a synchronized block and a volatile read.

Answer 1

Yes, you still need the volatile write, according to Java Memory Model. There is no synchronization order from unlocking cache to a subsequent volatile read of cache: unlock -> volatileRead does not guarantee visibility. You need either unlock -> lock or volatileWrite -> volatileRead.

However, real JVMs have much stronger memory guarantees. Usually unlock and volatileWrite have the same memory effect (even if they are on different variables); same as lock and volatileRead.

So we have a dilemma here. The typical recommendation is that you should strictly follow the spec. Unless you have very broad knowledge of the matter. For example, a JDK code may employ some tricks that are not theoretically correct; but the code is targeting a specific JVM and the author is an expert.

The relative overhead of the extra volatile write doesn't seem to be that big anyway.

Your code is correct and efficient; however it's outside of typical patterns; I would tweak it a little bit like:

  private final    byte[][][] cacheF = new ...;  // dimensions fixed?
  private volatile byte[][][] cacheV = cacheF;

  public byte[] getData(int row, int col)
  {
    return cacheV[row][col];
  }

  public void updateData(int row, int col, byte[] data)
  {
    synchronized(cacheF)
    {
      if (!Arrays.equals(data,cacheF[row][col]))
      {
        cacheF[row][col] = data;

        cacheV = cacheF; 
      }
    }
  }

Answer 2

An index write to a volatile array actually has no memory effects. That is, if you have already instantiated the array, having the field declared as volatile won't give you the memory semantics your looking for when assigning to elements within the array.

In other words

private volatile byte[][]cache = ...;
cache[row][col] = data;

Has the same memory semantics as

private final byte[][]cache = ...;
cache[row][col] = data;

Because of this you must synchronize on all reads and writes to the array. And when I say 'same memory semantics' I mean there is no guarantee that threads will read the most up to date value of cache[row][col]