Java concurrent visibility of primitive array writes

Question

I recently found this gem in my code base:

/** This class is used to "publish" changes to a non-volatile variable.
 *
 * Access to non-volatile and volatile variables cannot be reordered,
 * so if you make changes to a non-volatile variable before calling publish,
 * they are guaranteed to be visible to a thread which calls syncChanges
 *
 */
private static class Publisher {
    //This variable may not look like it's doing anything, but it really is.
    //See the documentaion for this class.
    private volatile AtomicInteger sync = new AtomicInteger(0);

    void publish() {
        sync.incrementAndGet();
    }

    /**
     *
     * @return the return value of this function has no meaning.
     * You should not make *any* assumptions about it.
     */
    int syncChanges() {
        return sync.get();
    }
}

This is used as such:

Thread 1

float[][] matrix;
matrix[x][y] = n;
publisher.publish();

Thread 2

publisher.syncChanges();
myVar = matrix[x][y];

Thread 1 is a background updating thread that runs continuously. Thread 2 is a HTTP worker thread that does not care that what it reads is in any way consistent or atomic, only that the writes "eventually" get there and are not lost as offerings to the concurrency gods.

Now, this triggers all my warning bells. Custom concurrency algorithm written deep inside of unrelated code.

Unfortunately, fixing the code is not trivial. The Java support for concurrent primitive matrices is not good. It looks like the clearest way to fix this is using a ReadWriteLock, but that would probably have negative performance implications. Correctness is more important, clearly, but it seems like I should prove that this is not correct before just ripping it out of a performance sensitive area.

According to the java.util.concurrent documentation, the following create happens-before relationships:

Each action in a thread happens-before every action in that thread that comes later in the program's order.

A write to a volatile field happens-before every subsequent read of that same field. Writes and reads of volatile fields have similar memory consistency effects as entering and exiting monitors, but do not entail mutual exclusion locking.

So it sounds like:

• matrix write happens-before publish() (rule 1)
• publish() happens-before syncChanges() (rule 2)
• syncChanges() happens-before matrix read (rule 1)

So the code indeed has established a happens-before chain for the matrix.

But I'm not convinced. Concurrency is hard, and I'm not a domain expert. What have I missed? Is this indeed safe?

Answer 1

In terms of visibility, all you need is volatile write-read, on any volatile field. This would work

final    float[][] matrix  = ...;
volatile float[][] matrixV = matrix;

Thread 1

matrix[x][y] = n;
matrixV = matrix; // volatile write

Thread 2

float[][] m = matrixV;  // volatile read
myVar = m[x][y];

or simply
myVar = matrixV[x][y];

But this is only good for updating one variable. If writer threads are writing multiple variables and the read thread is reading them, the reader may see an inconsistent picture. Usually it's dealt with by a read-write lock. Copy-on-write might be suitable for some use patterns.

Doug Lea has a new "StampedLock" http://gee.cs.oswego.edu/dl/jsr166/dist/jsr166edocs/jsr166e/StampedLock.html for Java8, which is a version of read-write lock that's much cheaper for read locks. But it is much harder to use too. Basically the reader gets the current version, then go ahead and read a bunch of variables, then check the version again; if the version hasn't changed, there was no concurrent writes during the read session.