Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

java: `volatile` private fields with getters and setters

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.

like image 627
khachik Avatar asked Jun 12 '12 12:06

khachik


People also ask

Can getters and setters be private Java?

The reason for declaring the getters and setters private is to make the corresponding part of the object's abstract state (i.e. the values) private. That's largely independent of the decision to use getters and setters or not to hide the implementation types, prevent direct access, etc.

Should setters and getters be private?

Usually you want setters/getters to be public, because that's what they are for: giving access to data, you don't want to give others direct access to because you don't want them to mess with your implementation dependent details - that's what encapsulation is about.

Does it requires to synchronize volatile getter and setter methods?

Getters and setters should be synchronized in pairs.

Do getters () and setters () require synchronization?

To elaborate, if one wants to obtain the most recent version of value in a multi-threaded application, both the getter and setter should be synchronized.


1 Answers

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.

like image 56
Joonas Pulakka Avatar answered Oct 04 '22 23:10

Joonas Pulakka