Re-ordering of assignments and adding a fence

Question

The following Java code looks a little strange because I have simplified it down to the bare essentials. I think the code has an ordering problem. I am looking at the first table in the JSR-133 Cookbook and it seems the normal store can be reordered with the volatile store in change().

Can the assignment to m_normal in change() move ahead of the assignment of m_volatile? In other words, can get() return null?

What is the best way to solve this?

private          Object m_normal   = new Object();
private volatile Object m_volatile;

public void change() {
    Object normal;

    normal = m_normal;      // Must capture value to avoid double-read

    if (normal == null) {
        return;
    }

    m_volatile = normal;
    m_normal   = null;
}

public Object get() {
    Object normal;

    normal = m_normal;      // Must capture value to avoid double-read

    if (normal != null) {
        return normal;
    }

    return m_volatile;
}

Note: I do not have control over the code where m_normal is declared.

Note: I am running on Java 8.

Answer 1

TL;DR: Friends do not let friends waste time with figuring out if racy accesses work up to Extreme Concurrency Optimist desires. Use volatile, and sleep happily.

I am looking at the first table in the JSR-133 Cookbook

Please note the full title is "JMM Cookbook For Compiler Writers". Which begs the question: are we compiler writers here, or just the users trying to figure out our code? I think the latter, so we should really close the JMM Cookbook, and open the JLS itself. See "Myth: JSR 133 Cookbook Is JMM Synopsis" and the section after that.

In other words, can get() return null?

Yes, trivially by get() observing the default values for the fields, without observing anything that change() did. :)

But I guess the question is, would it be allowed to see the old value in m_volatile after change() completed (Caveat: for some notion of "completed", because that implies time, and logical time is specified by JMM itself).

The question is basically, is there a valid execution that includes read(m_normal):null --po/hb--> read(m_volatile):null, with the read of m_normal observing the write of null to m_normal? Yes, here it is: write(m_volatile, X) --po/hb--> write(m_normal, null) ... read(m_normal):null --po/hb--> read(m_volatile):null.

The reads and writes to m_normal are not ordered, and so there is no structural constraints that prohibit the execution that reads both nulls. But "volatile", you would say! Yes, it comes with some constraints, but it is in wrong order w.r.t. the non-volatile operations, see "Pitfall: Acquiring and Releasing in Wrong Order" (look at that example closely, it is remarkably similar to what you are asking).

It is true that the operations on m_volatile itself are providing some memory semantics: the write to m_volatile is "release" that "publishes" everything happened before it, and the read from m_volatile is the "acquire" that "gets" everything published. If you do the derivation like in this post accurately, the pattern appears: you can trivially move the operations over the "release" program-upwards (those were racy anyway!), and you can trivially move the operations over the "acquire" program-downwards (those were racy anyway too!).

This interpretation is frequently called "roach motel semantics", and gives the intuitive answer to: "Can these two statements reorder?"

m_volatile = value; // release
m_normal   = null;  // some other store

The answer under roach motel semantics is "yes".

What is the best way to solve this?

The best way to solve is to avoid racy operations to begin with, and thus avoid the whole mess. Just make m_normal volatile, and you are all set: the operations over both m_normal and m_volatile would be sequentially consistent.

Would adding value = m_volatile; after m_volatile = value; prevent the assignment of m_normal happening before the assignment of m_volatile?

So the question is, would this help:

m_volatile = value; // "release"
value = m_volatile; // poison "acquire" read 
m_normal   = null;  // some other store

In naive world of only roach motel semantics, it could help: it would seem as if poison acquire breaks the code movement. But, since the value of that read is unobserved, it is equivalent to the execution without any poison read, and good optimizers would exploit that. See "Wishful Thinking: Unobserved Volatiles Have Memory Effects". It is important to understand that volatiles do not always mean barriers, even if the conservative implementation outlined in JMM Cookbook for Compiler Writers has them.

Aside: there is an alternative, VarHandle.fullFence() that can be used in the example like this, but it is restricted to very powerful users, because reasoning with barriers gets borderline insane. See "Myth: Barriers Are The Sane Mental Model" and "Myth: Reorderings And Commit to Memory".

Just make m_normal volatile, and everyone would sleep better.