java: `volatile` private fields with getters and setters

Question

Should we declare the private fields as volatile if the instanced are used in multiple threads?

In Effective Java, there is an example where the code doesn't work without volatile:

import java.util.concurrent.TimeUnit;

// Broken! - How long would you expect this program to run?
public class StopThread {
    private static boolean stopRequested; // works, if volatile is here

    public static void main(String[] args) throws InterruptedException {
        Thread backgroundThread = new Thread(new Runnable() {
            public void run() {
                int i = 0;
                while (!stopRequested)
                    i++;
            }
        });
        backgroundThread.start();
        TimeUnit.SECONDS.sleep(1);
        stopRequested = true;
    }
}

The explanations says that

while(!stopRequested)
    i++;

is optimized to something like this:

if(!stopRequested)
    while(true)
        i++;

so further modifications of stopRequested aren't seen by the background thread, so it loops forever. (BTW, that code terminates without volatile on JRE7.)

Now consider this class:

public class Bean {
    private boolean field = true;

    public boolean getField() {
        return field;
    }

    public void setField(boolean value) {
        field = value;
    }
}

and a thread as follows:

public class Worker implements Runnable {
    private Bean b;

    public Worker(Bean b) {
        this.b = b;
    }

    @Override
    public void run() {
        while(b.getField()) {
            System.err.println("Waiting...");
            try { Thread.sleep(1000); }
            catch(InterruptedException ie) { return; }
        }
    }
}

The above code works as expected without using volatiles:

public class VolatileTest {
    public static void main(String [] args) throws Exception {
        Bean b = new Bean();

        Thread t = new Thread(new Worker(b));
        t.start();
        Thread.sleep(3000);

        b.setField(false); // stops the child thread
        System.err.println("Waiting the child thread to quit");
        t.join();
        // if the code gets, here the child thread is stopped
        // and it really gets, with JRE7, 6 with -server, -client
    }
}

I think because of the public setter, the compiler/JVM should never optimize the code which calls getField(), but this article says that there is some "Volatile Bean" pattern (Pattern #4), which should be applied to create mutable thread-safe classes. Update: maybe that article applies for IBM JVM only?

The question is: which part of JLS explicitly or implicitly says that private primitive fields with public getters/setters must be declared as volatile (or they don't have to)?

Sorry for a long question, I tried to explain the problem in details. Let me know if something is not clear. Thanks.

Answer 1

Before I answer your question I want to address

BTW, that code terminates without volatile on JRE7

This can change if you were to deploy the same application with different runtime arguments. Hoisting isn't necessarily a default implementation for JVMs so it can work in one and not in another.

To answer your question there is nothing preventing the Java compiler from executing your latter example like so

@Override
public void run() {
    if(b.getField()){
        while(true) {
            System.err.println("Waiting...");
            try { Thread.sleep(1000); }
            catch(InterruptedException ie) { return; }
        }
    }
}

It is still sequentially consistent and thus maintains Java's guarantees - you can read specifically 17.4.3:

Among all the inter-thread actions performed by each thread t, the program order of t is a total order that reflects the order in which these actions would be performed according to the intra-thread semantics of t.

A set of actions is sequentially consistent if all actions occur in a total order (the execution order) that is consistent with program order, and furthermore, each read r of a variable v sees the value written by the write w to v such that:

In other words - So long as a thread will see the read and write of a field in the same order regardless of the compiler/memory re ordering it is considered sequentially consistent.

Answer 2

The question is: which part of JLS explicitly or implicitly says that private primitive fields with public getters/setters must be declared as volatile (or they don't have to)?

The JLS memory model doesn't care about getters/setters. They're no-ops from the memory model perspective - you could as well be accessing public fields. Wrapping the boolean behind a method call doesn't affect its memory visibility. Your latter example works purely by luck.

Should we declare the private fields as volatile if the instanced are used in multiple threads?

If a class (bean) is to be used in multithreaded environment, you must somehow take that into account. Making private fields volatile is one approach: it ensures that each thread is guaranteed to see the latest value of that field, not anything cached / optimized away stale values. But it doesn't solve the problem of atomicity.

The article you linked to applies to any JVM that adheres to the JVM specification (which the JLS leans on). You will get various results depending on the JVM vendor, version, flags, computer and OS, the number of times you run the program (HotSpot optimizations often kick in after the 10000th run) etc, so you really must understand the spec and carefully adhere to the rules in order to create reliable programs. Experimenting in this case is a poor way to find out how things work because the JVM can behave in any way it wants as long at it falls within the spec, and most JVMs do contain loads of all kind of dynamic optimizations.

Answer 3

No, that code is just as incorrect. Nothing in the JLS says a field must be declared as volatile. However, if you want your code to work correctly in a multi-threaded environment, then you have to obey the visibility rules. volatile and synchronized are two of the major facilities for correctly making data visible across threads.

As for your example, the difficulty of writing multi-threaded code is that many forms of incorrect code work fine in testing. Just because a multi-threaded test "succeeds" in testing does not mean it is correct code.

For the specific JLS reference, see the Happens Before section (and the rest of the page).

Note, as a general rule of thumb, if you think you have come up with a clever new way to get around "standard" thread-safe idioms, you are most likely wrong.