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

You are correctly mentioned the rule #2 of happens-before relationship

A write to a volatile field happens-before every subsequent read of that same field.

However, it doesn't guarantee that publish() will ever be called before syncChanges() on the absolute timeline. Lets change your example a bit.

Thread 1:

matrix[0][0] = 42.0f;
Thread.sleep(1000*1000); // assume the thread was preempted here
publisher.publish(); //assume initial state of sync is 0 

Thread 2:

int a = publisher.syncChanges();
float b = matrix[0][0];

What are the options for a and b variables are available ?

• a is 0, b can be 0 or 42
• a is 1, b is 42 because of the happens-before relationship
• a is greater than 1 (Thread 2 was slow for some reason and Thread 1 was lucky to publish updates several times), value of b depends on the business logic and the way matrix is handled - does it depend on the previous state or not?

How to deal with it? It depends on the business logic.

• If Thread 2 polls the state of a matrix from time to time and it's perfectly fine to have some outdated values in between, if in the end the correct value will be processed, then leave it as is.
• If Thread 2 doesn't care about missed updates but it always wants to observe up-to-date matrix then use copy-on-write collections or use ReaderWriteLock as it was mentioned above.
• If Thread 2 does care about single updates then it should be handled in a smarter way, you might want to consider wait() / notify() pattern and notify Thread 2 whenever matrix is updated.

Answer 2

This does look safe for publishing single updates to the matrix, but of course it doesn't provide any atomicity. Whether that's okay depends on your application, but it should probably be documented in a utility class like this.

However, it contains some redundancy and could be improved by making the sync field final. The volatile access of this field is the first of two memory barriers; by contract, calling incrementAndGet() has the same effect on memory as a write and a read on a volatile variable, and calling get() has the same effect as a read.

So, the code can rely on the synchronization provided by these methods alone, and make the field itself final.

Answer 3

Using volatile is not a magic bullet to synch everything. It is guaranteed that if another thread reads the updated value of a volatile variable, they will also see every change made to a non-volatile-variable before that. But nothing guarantees that the other thread will read the updated value.

In the example code, if you make several writes to matrix and then call publish(), and the other thread calls synch() and then reads the matrix, then the other thread may see some, all, or none of the changes:

• All the changes, if it reads the updated value from publish()
• None of the changes, if it reads the old published value and none of the changes have leaked through
• Some of the changes, if it reads the previously published value, but some of the changes have leaked through

See this article

Answer 4

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.